These are chat archives for dry-rb/chat

8th
Jul 2016
Samson Ootoovak
@ootoovak
Jul 08 2016 00:48
Ok I am having a very weird issue that I think is a Dry-Types bug. Would appreciate info if I am wrong and if so how I might fix it. I'll put the error case below:
require "dry-types"

class HeaderRecord < Dry::Types::Struct
  module Types; include Dry::Types.module; end
  attributes number: Types::Coercible::Int
end

HeaderRecord.new(number: "00000004")
# => #<HeaderRecord number=4>

HeaderRecord.new(number: "00000058")
# =>
# ArgumentError: invalid value for Integer(): "00000058"
# from /Users/ootoovak/.gem/ruby/2.2.3/gems/dry-types-0.8.0/lib/dry/types/constructor.rb:28:in `Integer'
Samson Ootoovak
@ootoovak
Jul 08 2016 01:05
Well aparently it is nothing to do with dry-types
# ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]

Integer("00000004") #=> 4

Integer("00000058") #=> ArgumentError: invalid value for Integer(): "00000058"

# Ruby ¯\_(ツ)_/¯
Daniel Sandbecker
@daniels
Jul 08 2016 08:48
@ootoovak When the string starts with a 0 and the next char is not b (binary) or x (hex) the string is presumed to be in octal representation. Hence 8 and 9 are not valid. Not super clear to me either, but there is this in the documentation for Kernel.Integer: "If arg is a String, when base is omitted or equals zero, radix indicators (0, 0b, and 0x) are honored."
Piotr Solnica
@solnic
Jul 08 2016 09:12
@ootoovak Coercible types use built-in coercions from Kernel, if you need something more dedicated use Form or JSON types
Nikita Shilnikov
@flash-gordon
Jul 08 2016 09:14
@solnic we also could add base arg to #Integer call
Piotr Solnica
@solnic
Jul 08 2016 09:17
yeah we could easily make this configurable
teju_chile
@tejaswinichile
Jul 08 2016 10:20
I just started reading about dry-validation, can anyone explain how its better/faster than ActiveRecordValidation
Chris Richards
@cmrichards
Jul 08 2016 10:40
teju_chile
@tejaswinichile
Jul 08 2016 10:49
I am reading the same, but there is no explanation about how does it handle it..
Oskar Szrajer
@gotar
Jul 08 2016 10:50
how it was achived you can check in source code :)
teju_chile
@tejaswinichile
Jul 08 2016 10:52
Piotr Solnica
@solnic
Jul 08 2016 12:33
@tejaswinichile lots of reasons…first of all it’s more flexible, you can validate any type of data regardless of the structure, attr names, value types etc.
ie try to use AM::V against data that have attributes called “validate” or “errors” :)
teju_chile
@tejaswinichile
Jul 08 2016 12:35
@solnic : thank you, that's more helpful
Piotr Solnica
@solnic
Jul 08 2016 12:35
it’s faster/simpler due to the way it works - it executes rules in a logical way w/o parsing options at run-time + its error processing is faster due to caching
there’s still a lot to improve/speed up though, ie dry-v builds success results too by default, which makes it sluggish for valid data (still not slower than AM::V though despite that)
I suspect we’re gonna make it 2-3x faster for 1.0 though
of course performance might not be the reason to use it :) I believe the biggest advantage over AM::V is its flexibility and strictness, you can really treat it as a tool for adding type-safety to your system, which safes you from a lot of silly bugs and reduces complexity of your code
and probably the biggest disadvantage right now is that it’s unstable, we’re getting closer to 1.0.0 each month though :)
teju_chile
@tejaswinichile
Jul 08 2016 12:48
wooow.. :) thank you again
pushing this thing later today
Fran Worley
@fran-worley
Jul 08 2016 14:44
@solnic any chance my PR can go into 0.8.1 ?
Piotr Solnica
@solnic
Jul 08 2016 14:45
checking it now
huh this honestly deserves 0.9.0 bump O_o
Fran Worley
@fran-worley
Jul 08 2016 14:47
I thought it might... can't really change message keys in a patch
Scott Eckenrode
@selectport
Jul 08 2016 14:55
@solnic … I assume #204 dry-rb/dry-validation#204 isn’t going to make 0.8.1 ? (no issues, just planning)
Piotr Solnica
@solnic
Jul 08 2016 14:57
@fran-worley I so rebased your branch
Fran Worley
@fran-worley
Jul 08 2016 14:57
everything broke?
Piotr Solnica
@solnic
Jul 08 2016 14:57
@fran-worley zero impact on performance AND it works better, great job :)
Fran Worley
@fran-worley
Jul 08 2016 14:58
Yey :)
Piotr Solnica
@solnic
Jul 08 2016 14:58
merging in
Fran Worley
@fran-worley
Jul 08 2016 14:58
This message was deleted
This message was deleted
This message was deleted
Go for it, it is there :)
Piotr Solnica
@solnic
Jul 08 2016 15:01
hah ok :)
Fran Worley
@fran-worley
Jul 08 2016 15:02
Oh but you can delete this
lib/dry/validation/predicate_args_hash.rb
Piotr Solnica
@solnic
Jul 08 2016 15:02
uhm ok
Fran Worley
@fran-worley
Jul 08 2016 15:02
As in delete the file. it's not used and I forgot to sync that change
Piotr Solnica
@solnic
Jul 08 2016 15:02
done
@selectport yeah I haven’t done this yet, it needs some thinking…for now pls stick to rules: s1.rules + s2.rules
Scott Eckenrode
@selectport
Jul 08 2016 15:03
@solnic thanks. no worries … will do
Piotr Solnica
@solnic
Jul 08 2016 15:04
I was thinking about an interface like use Schema1, Schema2 maybe that’s gonna be nicer
Scott Eckenrode
@selectport
Jul 08 2016 15:04
@solnic - for the nested case, too ?
Piotr Solnica
@solnic
Jul 08 2016 15:06
@fran-worley uhm, did you just push again to that branch?
Fran Worley
@fran-worley
Jul 08 2016 15:07
Not intentionally, I was synchronised my local repo and it did that
Piotr Solnica
@solnic
Jul 08 2016 15:07
damn
Fran Worley
@fran-worley
Jul 08 2016 15:07
Sorry
Piotr Solnica
@solnic
Jul 08 2016 15:07
I’ll reset —hard and push again
no worries!
Fran Worley
@fran-worley
Jul 08 2016 15:08
I've got to get better with Git.
Piotr Solnica
@solnic
Jul 08 2016 15:08
git push —force exists for a reason :laughing:
@selectport it can’t work the way you want with CountrySchema since you are defining key in both places
what’s the reason why you want it to be like that?
Scott Eckenrode
@selectport
Jul 08 2016 15:12
@solnic - my issue is really just the flat - was asking about the nested to see if they were going to change to the Use Schema1, Schema2 (ie. a change) so both nested and flat were treated consistently ?
my end goal is just to be able to reuse ‘field-level’ schemas across several places in the app … but they aren’t ‘nested’ - they are single field keys
Piotr Solnica
@solnic
Jul 08 2016 15:14
required(:country).schema(CountrySchema) will never work when CountrySchema already defines a rule for :countrykey
just inherit rules from CountrySchema and don’t define required(:country) twice
Scott Eckenrode
@selectport
Jul 08 2016 15:14
but isn’t my issue that i cannot define CountrySchema unless its a nested?
Piotr Solnica
@solnic
Jul 08 2016 15:15
@fran-worley could you update CHANGELOG in master pls? :)
Fran Worley
@fran-worley
Jul 08 2016 15:16
yup
Piotr Solnica
@solnic
Jul 08 2016 15:17
@selectport no, there’s no issue other than doing required(:country).schema(CountrySchema)
just do:
Dry::Validation.Schema(rules: CountrySchema.class.rules) do
  required(:city).filled

  required(:location).schema(LocationSchema)
end
LocationSchema defines keys and you want to nest them under :location but you’re not doing that with :country because in that case you just want to inherit rules
we could make that look nicer via use location: LocationSchema, CountrySchema or sth similar
as a shortcut interface
Scott Eckenrode
@selectport
Jul 08 2016 15:21
@solnic - trying that - gimme a second …. that’s not what i tried before….
Scott Eckenrode
@selectport
Jul 08 2016 15:29
@solnic … i’m close but not there yet ….

```AgeSchema = Dry::Validation.Schema do
configure { config.type_specs = true }
required(:age, :int) { filled? & int? & gt?(40) & lt?(50) }
end

NameSchema = Dry::Validation.Schema do
configure { config.type_specs = true }
required(:name, :string).filled(:str?, min_size?: 5)
end

PersonValidator = Dry::Validation.Schema do
configure { config.type_specs = true }
required(:name, :string).schema(NameSchema)
required(:age, :int).schema(AgeSchema)
optional(:state, :string).maybe(:str?, max_size?: 2)
end```

argh
Piotr Solnica
@solnic
Jul 08 2016 15:30
:D
Scott Eckenrode
@selectport
Jul 08 2016 15:31
AgeSchema = Dry::Validation.Schema do
    configure { config.type_specs = true }
    required(:age, :int) { filled? & int?  & gt?(40) & lt?(50) }
end

NameSchema = Dry::Validation.Schema do
    configure { config.type_specs = true }
    required(:name, :string).filled(:str?, min_size?: 5)
end

PersonValidator = Dry::Validation.Schema do
  configure { config.type_specs = true }
  required(:name, :string).schema(NameSchema)
  required(:age, :int).schema(AgeSchema)
  optional(:state, :string).maybe(:str?, max_size?: 2)
end
sorry
Piotr Solnica
@solnic
Jul 08 2016 15:32
Dry::Validation.Schema(rules: AgeSchema.class.rules + NameSchema.class.rules)
and remove :name and :age from PersonValidator
because you inherit these rules
Scott Eckenrode
@selectport
Jul 08 2016 15:33
hmmm … ok. that kind of obfuscates but i can live with that !
Piotr Solnica
@solnic
Jul 08 2016 15:33
oh wait, that’s not gonna work with type-specs yet
so disable them for now
that feature is still a WIP
Chris Richards
@cmrichards
Jul 08 2016 15:34
This message was deleted
Scott Eckenrode
@selectport
Jul 08 2016 15:34
no problem ! i can refactor later
Chris Richards
@cmrichards
Jul 08 2016 15:34
This message was deleted
Piotr Solnica
@solnic
Jul 08 2016 15:35
@cmrichards :)
Scott Eckenrode
@selectport
Jul 08 2016 15:35
@solnic - should i close #204 ?
Piotr Solnica
@solnic
Jul 08 2016 15:35
nope! let’s leave this open so we can discuss things there once I start working on some improvements that are related to this
Scott Eckenrode
@selectport
Jul 08 2016 15:35
@solnic - cool … thank you once again !
Chris Richards
@cmrichards
Jul 08 2016 15:36
do all the dry-rb gems work fine with rails 5?
Piotr Solnica
@solnic
Jul 08 2016 15:36
I have no idea :smile:
Chris Richards
@cmrichards
Jul 08 2016 15:37
You know you love Rails really ;-)
Piotr Solnica
@solnic
Jul 08 2016 15:37
but we made a couple of really important changes that should fix code reloading issues
specifically, Struct/Value in dry-types no longer register themselves in types container so I think if you include dry-types-rails in your project things should work
other than that, I don’t expect problems unless some monkey-patch bites you
Chris Richards
@cmrichards
Jul 08 2016 15:39
I have a couple of places in the main rails 2 app that I work on where it raises an exception only the second time you re-load a page in development mode. I have no clue how to fix it and it doesn't happen in production mode. I just leave it there. I feel bad. It's a problem with a missing association (which is there).
Piotr Solnica
@solnic
Jul 08 2016 15:40
@cmrichards are you using latest versions?
Chris Richards
@cmrichards
Jul 08 2016 15:40
Latest version of rails 2, yes.
Piotr Solnica
@solnic
Jul 08 2016 15:40
asking about dry stuff :)
Chris Richards
@cmrichards
Jul 08 2016 15:41
sorry, i was just having a random moan about code reloading in Rails 2. Nothing to do with dry stuff
Piotr Solnica
@solnic
Jul 08 2016 15:41
ah ok
@fran-worley I bumped to 0.9.0 - too many changes/improvements/fixes for a bug-fix release
so we’re gonna have 0.10.0 after all ¯_(⊙ʖ⊙)
Fran Worley
@fran-worley
Jul 08 2016 15:43
you need to stop working so hard!!
Piotr Solnica
@solnic
Jul 08 2016 15:45
that’s true O_o
Piotr Solnica
@solnic
Jul 08 2016 16:13
@backus ping
Fran Worley
@fran-worley
Jul 08 2016 16:22
@solnic to what extent are messages cached? If I call schema.messages and schema.messages(full: true)
what is the impact on performance ?
Piotr Solnica
@solnic
Jul 08 2016 16:23
oh just the message templates
and all hints but that’s safe
hints are not variables
so ie if you use I18n we’re gonna fetch message templates just once
Fran Worley
@fran-worley
Jul 08 2016 16:28
So if I call messages and then messages(full: true) It will run the compilers twice?
Piotr Solnica
@solnic
Jul 08 2016 16:31
@fran-worley yes
I guess that’s stupid :laughing:
Fran Worley
@fran-worley
Jul 08 2016 16:33
Especially as in our case we run the two calls without changing anything
couldn't :full look for the raw messages, if there then inject the name and if not then run the compiler ?
Piotr Solnica
@solnic
Jul 08 2016 16:49
hmm
I mean…you only get messages once
so it’s either :full or not
Fran Worley
@fran-worley
Jul 08 2016 16:53
in reform we build our own errors object off the back of dry-validation and support the active model style methods error_messages and full_error_messages. To so that we end up calling both message types on a schema
I might move full_error_messages to a module that you have to opt into...
Piotr Solnica
@solnic
Jul 08 2016 16:54
full_error_messages is an array right?
Fran Worley
@fran-worley
Jul 08 2016 16:55
hash
Piotr Solnica
@solnic
Jul 08 2016 16:55
I think you could leverage the MessageSet API I added in 0.9.0
it’s not exposed, but I can make it exposed now
one sec, I’ll show you an example
Piotr Solnica
@solnic
Jul 08 2016 17:00
irb(main):002:0> msg_set = Dry::Validation.Schema { required(:name).filled }.(name: nil).messages(as_hash: false)
=> #<Dry::Validation::MessageSet:0x007fa74928d5f0 @messages=[#<Dry::Validation::Message predicate=:filled? path=[:name] text="must be filled" options={:args=>[], :rule=>:name, :each=>false}>], @hints={}, @paths=[[:name]], @placeholders={:name=>[]}>
irb(main):003:0> msg_set.to_h
=> {:name=>[“must be filled"]}
@fran-worley ^^ so, that’s what we have under the hood (I added as_hash option locally just to show this)
now, what we could do is to give message objects access to translating rule names, so that you could dump them to full messages
this means reform could simply ask for message-set object and a) dump it to hash b) dump it to hash in :full mode
this way we compile messages just once
Fran Worley
@fran-worley
Jul 08 2016 17:03
That would be perfect
exactly what I was hoping for. Also is there a way to inject a prefix key into the message set?
Piotr Solnica
@solnic
Jul 08 2016 17:03
prefix key?
Fran Worley
@fran-worley
Jul 08 2016 17:05
so reform defines all it's validations on the root i.e. required(:name).filled, but rails expects to see the error at model.field: ['some messages'] so we inject the prefix
Piotr Solnica
@solnic
Jul 08 2016 17:06
uhm, why is that needed in rails?
Fran Worley
@fran-worley
Jul 08 2016 17:06
because their form_for helper prefixes all attributes with the name of the model
e.g. comments_title comments_author_email
Piotr Solnica
@solnic
Jul 08 2016 17:07
oh boy
Fran Worley
@fran-worley
Jul 08 2016 17:07
I know
fab right !!
No worries if it's a rails only use case, happy to leave the prefix injection in Reform just wondered if you'd come across it anywhere
Piotr Solnica
@solnic
Jul 08 2016 17:08
I did not
Fran Worley
@fran-worley
Jul 08 2016 17:08
cool well the message set thing will be a massive improvement on what we have now
Piotr Solnica
@solnic
Jul 08 2016 17:08
fwiw we have a MUCH BETTER foundation now
but generating a potentially deeply nested hash ain’t trivial
with a flat structure adding a prefix is 0 effort
but I wonder what should we do with nested stuff
Fran Worley
@fran-worley
Jul 08 2016 17:09
Piotr Solnica
@solnic
Jul 08 2016 17:09
I gotta think about this
right, there’s quite a bit of nonsense in errors in Rails
like this magic :base thing
try to have a :base attribute there
I wonder what would that cause
Fran Worley
@fran-worley
Jul 08 2016 17:11
At the moment we loop through the dry-v errors object for each schema (yes each validation group is it's own schema) and then add your message to ours
that way we can combine the messages for multiple schemas and inject the stupid prefix thing
Piotr Solnica
@solnic
Jul 08 2016 17:11
well if you have the assumption that it’s always a flat structure
or in other words, that each error message set is for the same model
then prefixing is gonna be simple
Fran Worley
@fran-worley
Jul 08 2016 17:12
I doubt we can guarantee it's gunna be flat...
yeah we get nested models and for nested models and collections you still get the beautiful base injection
Piotr Solnica
@solnic
Jul 08 2016 17:15
I gotta postpone 0.9.0 btw, I just checked hanami-validations and a couple of tests started to fail
I can expose an API for returning plain message set too
we could add settings for schema so that you can configure default behavior
ie setting :messages_format
Fran Worley
@fran-worley
Jul 08 2016 17:16
Cool that would be awesome. :thumbsup:
Piotr Solnica
@solnic
Jul 08 2016 17:16
and config.message_format = :hash
or config.message_format = :message_set
or :array
with combinations like hash_full or array_full
Fran Worley
@fran-worley
Jul 08 2016 17:18
cool :)
Piotr Solnica
@solnic
Jul 08 2016 17:19
btw I’ve got a plan for making hints really really smart
with message objects and message set in place, we can add a couple of smart algos that will calculate which hints should be merged in
based on assigning priority to predicates
so ie filled? would be 0
and type-checks would be 1
and we could compare which ones have bigger priority
Fran Worley
@fran-worley
Jul 08 2016 17:21
how does that work with custom predicates?
Piotr Solnica
@solnic
Jul 08 2016 17:21
they would have big priority by default I think
so they are always displayed
but even that could be configurable :laughing:
Fran Worley
@fran-worley
Jul 08 2016 17:22
Well that sounds amazing. We should put these in git issues or they get forgotten...
Piotr Solnica
@solnic
Jul 08 2016 17:23
right now we exclude hints for type-checks but there are cases where you would like to have them included
Fran Worley
@fran-worley
Jul 08 2016 17:23
FYI I'm not ignoring the dry-logic ast change, I've just got to find some time :)
Piotr Solnica
@solnic
Jul 08 2016 17:23
ie required(:foo) { int? | str? }
right now this won’t generate any hints
Fran Worley
@fran-worley
Jul 08 2016 17:23
Ohh :(
Piotr Solnica
@solnic
Jul 08 2016 17:23
yep
it’s because in majority of the cases type-check happens first, so if it fails you’re gonna get an error
but then you have less common cases :D
otoh filled(:int?) is probably common, no hints in this case too
the trick is that you may have stuff like number? & int?
Fran Worley
@fran-worley
Jul 08 2016 17:27
then your hint should render the & and ors correctly in one message
Piotr Solnica
@solnic
Jul 08 2016 17:27
and with filled(:int?) we want to display “must be an integer” for an empty string rather than both about filled and integer
is ie filled? < int? and number? < :int? in terms of priorities, I think
Fran Worley
@fran-worley
Jul 08 2016 17:29
yeah that sounds right
Piotr Solnica
@solnic
Jul 08 2016 17:29
got a local branch that displays hints for type-checks but a couple of examples in specs are meh so I’d like to improve that
ie this: settings: { newsletter: [‘must be false', 'must be boolean'] }
UGH
again, priorities, bool? < false?
Fran Worley
@fran-worley
Jul 08 2016 17:30
yeah the more specific should have a higher priority
Piotr Solnica
@solnic
Jul 08 2016 17:30
so ie we could have bool? with prio 1 and true?/false? with 2
and filled? would be 0
so we can easily reject low-prio messages
exactly :)
and this was my goal since the beginning :)
as far as OR goes, this can be done rather easily now…
hints for ORs return a pair for each disjunction rule, so we could tweak the message/message set stuff so that when it sees an array rather than a message, it can create Message::Or object from the pair
and then we can convert it using an or message template (as we need to support I18n)
so in en it’d be %{left} or %{right}
Fran Worley
@fran-worley
Jul 08 2016 17:35
Yeah that would be perfect
Piotr Solnica
@solnic
Jul 08 2016 17:36
I’ll give that a shot next week
maybe it’ll be in 0.9.0
this should be really simple to do now (famous last words!!!)
Fran Worley
@fran-worley
Jul 08 2016 17:37
:)
I've got to head off now but it all sounds great :)
Piotr Solnica
@solnic
Jul 08 2016 17:37
cool :) have a good Friday evening then!
Fran Worley
@fran-worley
Jul 08 2016 17:37
you too :)
Piotr Solnica
@solnic
Jul 08 2016 20:27
@fran-worley so I pushed dry-v 0.9.0 after all, I added this as_hash option to #messages API (not documented) so you folks at Reform can play with it
@fran-worley just schema.({}).messages(as_hash: false)
also, you can finally define maybe-schemas (second paragraph) http://dry-rb.org/gems/dry-validation/nested-data/
also also, you can now define base schema classes for your app (second paragraph again) http://dry-rb.org/gems/dry-validation/basics/working-with-schemas/