Make it possible to coerce nil Array of Types to []

[stanhu]

Going back to our discussion in https://github.com/ruby-grape/grape/pull/2040#issuecomment-615483919, I've found it a bit tedious dealing with optional Array[Integer], Array[String], etc. types. Suppose I have an API with an optional parameter:

optional :values, type: Array[String], coerce_with: ->(val) { val.split(',').map(&:strip) }

I have four cases to consider for a PUT request:

Parameter provided	Value	Intended behavior
Y	nil	Clear all values
Y	[]	Clear all values
N	N/A	Leaving existing values alone
Y	["test", "foo"]	Update existing values

The first two cases are identical, but we have hundreds of API calls, and I'd rather not fix all the supporting code because nil is now a valid input.

For optional parameters, I can't use default: [] because this always includes the parameter, even though it's optional. Adding the default value will cause us to clear out the values when I only want to coerce the value if the parameter is provided.

In Grape v1.3.x, the coercion method is not run for nil, so I can't even manually coerce nil to some other value with a custom method. To upgrade to Grape v1.3.x, I have to do the coercion within the API handler with this helper function:

      def coerce_nil_param_to_array(params, key)
        params.tap do |params|
            params[key] = [] if params.key?(key) && params[key].nil?
        end
      end

For example:

        update_params = declared_params(include_missing: false)
        update_params = coerce_nil_param_to_array(declared_params, :values)

This isn't ideal; I could iterate through the options inside the route, but that doesn't feel right.

Now I'm wondering whether the coercion method should be run for nil types? Or make that an option?

[dblock]

Thanks for bringing this up. Let me repeat this back to make sure I understand. Everything works except when the caller provides nil, in which case the coercer is not invoked and you don't get back [], but nil, and you want [] so that values can be cleared?

Attempting to change that by setting a default value to [] doesn't work either because if the caller provides nil, then the default value is not applied by design?

The behavior above is consistent for all types and we can't have it both ways. Personally, I don't think optionally allowing both is a great idea, it just creates a whole other matrix of possibilities. This leads me to think that easiest fix is actually in your API code itself by wrapping usage of values in Array(params[:values]), which turns nil and any single non-array parameter into an Array.

2.6.6 :001 > Array(nil)
 => [] 

WDYT?

[stanhu]

Attempting to change that by setting a default value to [] doesn't work either because if the caller provides nil, then the default value is not applied by design?

@dblock Thanks for responding. The problem is that if there is default, the parameter is always included, so if the caller doesn't provide the parameter there is unintended consequence of falling into case 2 instead of case 3 where values is left alone.

This leads me to think that easiest fix is actually in your API code itself by wrapping usage of values in Array(params[:values]), which turns nil and any single non-array parameter into an Array.

Yeah, I'd still have to do that for every optional parameter in every API call that I have. It's doable, but just a bit tedious and error-prone. I'd much prefer to push this up into the DSL with a custom type if necessary. It looks like parse is also not called if the value is nil.

[dblock]

I see. Regarding the default that makes sense - there's a default value for when there's no value, no parameters, etc. It's ... the default.

Why do you want to clear all values when a parameter is not provided? Seems like the user of the API doesn't intend to do anything, then do nothing.

[stanhu]

Why do you want to clear all values when a parameter is not provided? Seems like the user of the API doesn't intend to do anything, then do nothing.

Note that the key is provided, but the value is nil. For example, both of these do the same thing, which clear out values:

1. PUT /myendpoint?values
2. PUT /myendpoint?values=

It's about preserving backwards compatibility since previous versions of Grape coerced nil to [], and I don't want to change the behavior of the above.

[dblock]

For both of these, is params.key?(:values) == true and when the user doesn't include values that's false? Then API could do this in a before block?

params[:values] ||= [] if params.key?(:values)

[stanhu]

@dblock Thanks. The problem is that I'd have to do something like that for every Array type, and it's easy to miss. This is how I dealt with this automatically:

    before do
      coerce_nil_params_to_array!
    end

And this is the helper:

      # Grape v1.3.3 no longer automatically coerces an Array
      # type to an empty array if the value is nil.
      def coerce_nil_params_to_array!
        keys_to_coerce = params_with_array_types

        params.each do |key, val|
          params[key] = Array(val) if val.nil? && keys_to_coerce.include?(key)
        end
      end

      def params_with_array_types
        options[:route_options][:params].map do |key, val|
          param_type = val[:type]
          # Search for parameters with Array types (e.g. "[String]", "[Integer]", etc.)
          if param_type =~ %r(\[\w*\])
            key
          end
        end.compact.to_set
      end

It feels like Grape should make this easier.

[dblock]

It feels like Grape should make this easier.

I agree.

[PervushinEugene]

I used types with NilClass optional :image_uids, types: [Array[String], NilClass], default: nil

[dblock]

I used types with NilClass optional :image_uids, types: [Array[String], NilClass], default: nil

If this works, should we be injecting NilClass every time the list of types has an Array and the keyword is optional? Someone wants to try to PR this and see whether any specs break?