Thanks for the quick response!
I believe I see what your suggestion is doing, looking at the deserialized
value if the attribute is string, but this doesn’t appear to solve the issue
that the serialization itself is raising an exception due to the string value
being invalid GeoJSON; as this step now happens before `validate` is even
called.
To test, I did replace the code with your suggestion, but still get the error
during serialization. The full stack trace, which shows the error happening
during _before_validation:
1) Citygram::Models::Event requires a valid GeoJSON feature geometry
Failure/Error: expect(event).not_to be_valid
NoMethodError:
undefined method `[]' for nil:NilClass
#
/home/vagrant/.rvm/gems/ruby-2.5.0/gems/georuby-2.3.0/lib/geo_ruby/geojson.rb:103:in
`parse_geometry'
#
/home/vagrant/.rvm/gems/ruby-2.5.0/gems/georuby-2.3.0/lib/geo_ruby/geojson.rb:120:in
`parse_geojson_feature'
#
/home/vagrant/.rvm/gems/ruby-2.5.0/gems/georuby-2.3.0/lib/geo_ruby/geojson.rb:94:in
`parse_geohash'
#
/home/vagrant/.rvm/gems/ruby-2.5.0/gems/georuby-2.3.0/lib/geo_ruby/geojson.rb:83:in
`parse'
# ./app/models.rb:65:in `block in <top (required)>'
# ./sequel/lib/sequel/plugins/serialization.rb:231:in `serialize_value'
# ./sequel/lib/sequel/plugins/serialization.rb:223:in `block in
serialize_deserialized_values'
# ./sequel/lib/sequel/plugins/serialization.rb:223:in `each'
# ./sequel/lib/sequel/plugins/serialization.rb:223:in
`serialize_deserialized_values'
# ./sequel/lib/sequel/plugins/serialization.rb:202:in `_before_validation'
# ./sequel/lib/sequel/plugins/timestamps.rb:71:in `_before_validation'
# ./sequel/lib/sequel/model/base.rb:1705:in `valid?'
# ./spec/models/event_spec.rb:29:in `block (2 levels) in <top (required)>'
# ./sequel/lib/sequel/database/transactions.rb:119:in `_transaction'
# ./sequel/lib/sequel/database/transactions.rb:100:in `block in
transaction'
# ./sequel/lib/sequel/database/connecting.rb:229:in `block in synchronize'
# ./sequel/lib/sequel/connection_pool/threaded.rb:104:in `hold'
# ./sequel/lib/sequel/database/connecting.rb:229:in `synchronize'
# ./sequel/lib/sequel/database/transactions.rb:89:in `transaction'
# ./spec/support/db_sandbox.rb:3:in `block (2 levels) in <top (required)>'
#
/home/vagrant/.rvm/gems/ruby-2.5.0/gems/sidekiq-3.5.1/lib/sidekiq/testing.rb:15:in
`__set_test_mode'
#
/home/vagrant/.rvm/gems/ruby-2.5.0/gems/sidekiq-3.5.1/lib/sidekiq/testing.rb:29:in
`fake!'
# ./spec/support/sidekiq.rb:9:in `block (2 levels) in <top (required)>'
Thank you again!
-Jesse
On 6/21/18, 3:44 PM,
"[email protected]<mailto:[email protected]> on behalf of
Jeremy Evans"
<[email protected]<mailto:[email protected]> on behalf of
[email protected]<mailto:[email protected]>> wrote:
On Thursday, June 21, 2018 at 1:38:56 PM UTC-7, [email protected] wrote:
Hi Jeremy,
Stumbled upon this issue when attempting to upgrade Sequel from 4.12.0 for
Citygram<https://github.com/codeforamerica/citygram>.
This project currently attempts to validate fields that are serialized as
GeoJSON before they are serialized via a Sequel plugin:
Validation:
https://github.com/codeforamerica/citygram/blob/b2c1faa513c570cf19a5da1aa8dcbd314cba7755/app/models/event.rb#L35
Plugin:
https://github.com/codeforamerica/citygram/blob/b2c1faa513c570cf19a5da1aa8dcbd314cba7755/lib/sequel/plugins/geometry_validation.rb
With the change you mentioned, which I tracked to
https://github.com/jeremyevans/sequel/commit/c61a63994, it does indeed attempt
to serialize as GeoJSON before the validation. This ends up resulting in more
opaque error messages as the exceptions bubble up from the serializer. I.e. we
get things like `undefined method `[]' for nil:NilClass` rather than `geom is
an invalid geometry`.
I understand why this change was made (to allow validation of the actual data
being inserted into the database), but do you (or anyone else :) have
suggestions for how we might preserve this error handling behavior?
I think the simplest approach would be to change validates_geometry to look at
@deserialized_values if geom is a String:
def validates_geometry(atts, opts = {})
atts.each do |att|
v = send(att)
if v.is_a?(String) && (dv = deserialized_values[att]) &&
!valid_geometry?(dv)
errors.add(att, opts[:message] || 'is an invalid geometry')
end
end
end
If you need to have validates_geometry work when not using the serialization,
that's a little more work, but not too much.
Thanks,
Jeremy
--
You received this message because you are subscribed to the Google Groups
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to
[email protected]<mailto:[email protected]>.
To post to this group, send email to
[email protected]<mailto:[email protected]>.
Visit this group at https://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups
"sequel-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sequel-talk.
For more options, visit https://groups.google.com/d/optout.
