cast from Array(String) to String failed from Avram::Params

[jwoertink]

In https://github.com/luckyframework/avram/pull/847 I had to change Avram::Params to take Hash(String, Array(String)) instead of Hash(String, String) so that we can allow array params to be passed in. This also aligns with URI::Params.

I wasn't able to make it a union because this caused a LOT of compiler headache trying to determine which type a value was, and constantly having to cast it.

To use this, you would do

params = Avram::Params({"username" => ["tacoguy"]})
SaveUser.create(params) do |_,_|
end

As it turns out, the specs never caught this because in the specs, we just inherit from Avram::Params

https://github.com/luckyframework/avram/blob/9c8caecf052641d15dbe027e0cafb1741d8af33f/spec/avram/params_spec.cr#L21-L37

The error actually borks on https://github.com/luckyframework/avram/blob/9c8caecf052641d15dbe027e0cafb1741d8af33f/src/avram/add_column_attributes.cr#L17

because in this case, value is Array(String) even though the column should just be String. The tricky part is typeof(value) here is actually Array(String) | String which ends up coming back to the original union issue. We need some sort of way to say that if value gets to this branch, and value is still an array, it needs to call .first, but if it's not an array, then make sure to cast as a String.

[jwoertink]

Ok, it turns out it's here https://github.com/luckyframework/avram/blob/9c8caecf052641d15dbe027e0cafb1741d8af33f/src/avram/params.cr#L29-L31

I'm just saying that the nested_arrays is the raw passed in hash. Getting the permitted keys we pull data twice from params

https://github.com/luckyframework/avram/blob/9c8caecf052641d15dbe027e0cafb1741d8af33f/src/avram/add_column_attributes.cr#L24-L26

when we merge them, if the nested_arrays is the raw hash, then it overrides the single_values.

Kinda like doing {"username" => "tacoguy"}.merge({"username" => ["tacoguy"]}).

[jwoertink]

So it seems that this doesn't affect normal operations...

This seems to work fine:

params = Avram::Params({"username" => ["tacoguy"]})
SignInUser.run(params) do |_,_|
end

[akadusei]

I'm running into the same issue. If it helps, here a trace:

cast from Array(String) to String failed, at /path/to/lucille/spec/support/app/src/models/user.cr:20:3:31 (TypeCastError)
         from spec/support/app/src/models/user.cr:20:3 in 'extract_changes_from_params'
         from spec/support/app/src/operations/create_user.cr:1:1 in 'set_attributes'
         from spec/support/app/src/operations/create_user.cr:1:1 in 'initialize'
         from spec/support/app/src/operations/create_user.cr:1:1 in 'new:active_at:inactive_at:id:email:level:metadata:height'
         from spec/support/app/src/operations/create_user.cr:1:1 in '->'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/example.cr:45:13 in 'internal_run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/example.cr:32:73 in '->'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/example/procsy.cr:16:15 in 'run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:365:11 in '->'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/example/procsy.cr:16:15 in 'run'
         from lib/avram/src/avram/spec_helper.cr:30:5 in 'wrap_spec_in_transaction'
         from spec/spec_helper.cr:12:1 in '->'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:71:26 in 'run_around_each_hook'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:66:7 in 'internal_run_around_each_hooks'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:59:7 in 'run_around_each_hooks'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:357:13 in 'run_around_each_hooks'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/example.cr:32:15 in 'run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:339:7 in 'run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:18:23 in 'internal_run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/context.cr:156:7 in 'run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/spec/dsl.cr:220:7 in '->'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/crystal/at_exit_handlers.cr:14:19 in 'run'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/crystal/main.cr:50:5 in 'exit'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/crystal/main.cr:45:5 in 'main'
         from /path/to/crenv/versions/1.4.0/share/crystal/src/crystal/main.cr:127:3 in 'main'
         from /lib/x86_64-linux-gnu/libc.so.6 in '__libc_start_main'
         from /path/to/.cache/crystal/crystal-run-spec.tmp in '_start'
         from ???

[jwoertink]

To get around this for now, I just inherited from Avram::Params, and overrode the methods to do what I needed.

# TODO: https://github.com/luckyframework/avram/issues/884
class GraphParams < Avram::Params
  def nested?(key : String) : Hash(String, String)
    @hash.select(&.ends_with?("[]").!).transform_values(&.first)
  end

  def nested_arrays?(key : String) : Hash(String, Array(String))
    @hash.select(&.ends_with?("[]")).transform_keys(&.chomp("[]"))
  end
end

GraphParams.new({
  "username" => ["tacoguy"],
  "meats[]" => ["pollo", "carne"]
})

It's a bit wonky having to add [] on to the key, but technically that's how it's passed through params anyway... but if I can get it to work without needing that, I'll try to figure that out. The main part is trying to decipher between what should be considered a single value, and what should be multiple values. This may end up just coming back to allowing a Union here of Hash(String, Array(String) | String), and then just dealing with casting .as(String) or .as(Array(String)) all over the place :stuck_out_tongue_closed_eyes:

[akadusei]

This seems to fix it:

1. Change Avram::AddColumnAttributes#extract_changes_from_params to:

   private def extract_changes_from_params
     permitted_params.each do |key, value|
       {% for attribute in attributes %}
         set_{{ attribute[:name] }}_from_param(value.as(Array(String))) if key == {{ attribute[:name].stringify }}
       {% end %}
     end
   end
   

   We don't need the is_a?(Generic) check, since we are already defining #set_{{ attribute[:name] }}_from_param conditionally.

2. Change Avram::AddColumnAttributes#set_{{ attribute[:name] }}_from_param to:

   # ...
   {% else %}
   def set_{{ attribute[:name] }}_from_param(_value : Array(String))
     _value = _value.first?
   
     # In nilable types, `nil` is ok, and non-nilable types we will get the
     # "is required" error.
     if _value.blank?
       {{ attribute[:name] }}.value = nil
       return
     end
   
     parse_result = {{ attribute[:type] }}.adapter.parse(_value.not_nil!)
   
     if parse_result.is_a? Avram::Type::SuccessfulCast
       {{ attribute[:name] }}.value = parse_result.value.as({{ attribute[:type] }})
     else
       {{ attribute[:name] }}.add_error "is invalid"
     end
   end
   {% end %}
   #...
   

I haven't tested this with the specs though, just fiddling with code in my lib/ directory.

[akadusei]

https://github.com/luckyframework/avram/blob/9c8caecf052641d15dbe027e0cafb1741d8af33f/src/avram/add_column_attributes.cr#L24

This should probably change too, to return an array with a single element:

single_values = @params.nested(self.class.param_key).reject {|k,v| k.ends_with?("[]")}.transform_values { |value| [value] }

And, just a question, shouldn't the .reject...ends_with? part be already handled by Lucky::Params?