Open thomaswitt opened 2 months ago
Adding this as a feature request.
We don't want to take a dependency on ActiveSupport since this is rails-agnostic and often used outside of the context of rails. However, this seems like reasonable behavior to support and I see two options to do so:
We can add a new configuration option to the map_attr
that allows you to specify the class used, eg: map_attr :my_data, klass: ActiveSupport::HashWithIndifferentAccess
. This requires explicit configuration to opt into.
We patch the MapMarshaler in aws-sdk-rails to automatically use HashWithIndifferentAccess. aws-sdk-rails already contains a number of convenience features that make using the SDK easier in the context of rails, so this might reasonably fit in that context.
The patch (which you could also apply in your own code) would look something like:
module IndifferentMapMarshaler
def type_cast(raw_value)
ActiveSupport::HashWithIndifferentAccess.new(super)
end
end
Aws::Record::Marshalers::MapMarshaler.extend(IndifferentMapMarshaler)
@alextwoods I would vote for Option 2 as it seems to me the more flexible Option - and it doesn't require configuration, which is an added bonus. Thanks for the patch and pointing out to the piece of code.
Just as a heads up, I am using that code since several months and it works really fine. So I can only encourage you to include it in the Gem.
One of the predominant use cases of this SDK is IMHO writing Rails applications. Therefore it would make sense that this SDK supports Rails conventions as much as much as possible. Unfortunately it doesn't do that currently in term of keys as symbols vs strings.
The desirable version
map_attr :my_data, default_value: { items: [] }
throws an error while, as the syntax required is
map_attr :my_data, default_value: { 'items' => [] }
.Same for ERB:
<% my_data.except(:name).keys.each do |key| %>
does not work but needs to be
<% my_data.except("name").keys.each do |key| %>
This is very un-rails-y, both in models as in views, as using HashWithIndifferentAccess gives you at least both options. If you don't want to support this out of the box due to the dependency to ActiveSupport, I would at least making an option when ActiveSupport is available (or automatically use it).