Bug #2893
closedPrevent symbols and fancy Hash subclasses from going in or out of the database in serialized attributes
Updated by Radhika Chippada over 10 years ago
Review comments:
1. How about the method name "has_symbols?" instead of "has_any_symbols?"
2. convert_serialized_symbols_to_strings method
This method checks "if attr.object_class == Hash"
Even though we have used only Hash type objects for serialized attributes, can't I have an array defined as seriazed attribute? For ex: "serialize :my_attr, Array"
Should this method hence also support Array objects as well? What if I said "serialize :my_attr, String" or some other db supported object?
Should this method intead omit this check and invoke "if self.class.has_any_symbols? attributes[colname]" for all cases or all supported cases?
3. Please add the below additional tests to ArvadosModelTest -> test "refuse symbol keys in serialized attribute: #{x.inspect}"
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb@ -32,6 +32,12
@ class ArvadosModelTest < ActiveSupport::TestCase
[ {:a => 'foo'},
+ {'a' => :foo},
+ {:a => ['foo', 'bar']},
+ {'a' => [:foo, 'bar']},
+ {:a => [:foo, :bar]},
+ {:a => {'foo' => {'bar' => 'baz'}}},
+ {'a' => {:foo => {'bar' => 'baz'}}},
{'a' => {'foo' => {:bar => 'baz'}}},
{'a' => {'foo' => {'bar' => :baz}}},
Updated by Tom Clegg over 10 years ago
- Target version changed from 2014-05-28 Pipeline Factory to 2014-06-17 Curating and Crunch
Updated by Tom Clegg over 10 years ago
Now at 15cc0bc
- Renamed to
has_symbols?
- Commented
convert_serialized_symbols_to_strings
- Convert all serialized types in
convert_serialized_symbols_to_strings
, not just Hash - Added test cases as suggested
- Improved readability of
has_symbols?
Updated by Anonymous over 10 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:21485541dec5b6df36aaba7d4c2a1e96ba65dec6.