feat: Avoid unexpected transformations for parameters

Restify checked every param if it responds to to_param, and calls it,
before passing values to Addressable::Template. Since Rails patches
to_param into anything, that resulted in accepting virtually anything
somehow into params.

For example, any Array was encoded a slash-delimited string of the
individual values ([1,2] -> "1/2"), which not only could result in
confusing things accidentially passed as params, but also made it
impossible to pass a parameter multiple times (a: [1, 2] -> "a=1&a=2").

This commit takes the basic type detection from Addressable::Template
and tries to only apply to_param, which addressable does not support at
all, for non-basic types. Therefore, arrays and hash, should behave
similar to when passed directly to Addressable::Template, but it will
still be possible to e.g. pass an ActiveRecord model as a parameter,
using #to_param.

This makes passing standard and Rails-style argument lists possible:

    expand(p: [1, 2])       -> "/?p=1&p=2"
    expand('p[]': [1, 2])   -> "/?p%5B%5D=1&p%5B%5D=2"

Fixes #44

lib/restify/relation.rb

@@ -77,9 +77,34 @@ def convert(params)
       end
     end
 
-    def convert_param(value)
-      return value.to_param.to_s if value.respond_to?(:to_param)
+    def convert_param(value, nesting: true)
+      # Convert parameters into values acceptable in a
+      # Addressable::Template, with some support for #to_param, but not
+      # for basic types.
+      if value == nil || # rubocop:disable Style/NilComparison
+         value.is_a?(Numeric) ||
+         value.is_a?(Symbol) ||
+         value.is_a?(Hash) ||
+         value == true ||
+         value == false ||
+         value.respond_to?(:to_str)
+        return value
+      end
+
+      # Handle array-link things first to *not* call #to_params on them,
+      # as that will concatenation any Array to "a/b/c". Instead, we
+      # want to check one level of basic types only.
+      if value.respond_to?(:to_ary)
+        return nesting ? value.to_ary.map {|val| convert_param(val, nesting: false) } : value
+      end
+
+      # Handle Rails' #to_param for non-basic types
+      if value.respond_to?(:to_param)
+        return value.to_param
+      end
 
+      # Otherwise, pass raw value to Addressable::Template and let it
+      # explode.
       value
     end
 

spec/restify/relation_spec.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'spec_helper'
+require 'active_support'
 
 describe Restify::Relation do
   subject(:relation) { described_class.new context, pattern }
@@ -28,12 +29,36 @@ def to_param
 
     it { is_expected.to be_a Addressable::URI }
 
+    context 'with nil' do
+      let(:params) { {id: nil} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/' }
+    end
+
+    context 'with false' do
+      let(:params) { {id: false} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/' }
+    end
+
+    context 'with true' do
+      let(:params) { {id: true} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/true' }
+    end
+
     context 'with #to_param object' do
       let(:params) { {id: cls_to_param.new} }
 
       it { expect(expanded.to_s).to eq 'http://test.host/resource/42' }
     end
 
+    context 'with array parameter' do
+      let(:params) { {id: [1, 2]} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/1,2' }
+    end
+
     context 'with unknown additional query parameter' do
       let(:pattern) { '/resource{?a,b}' }
       let(:params) { {a: 1, b: 2, c: 3} }
@@ -47,10 +72,52 @@ def to_param
       it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=cde' }
     end
 
+    context 'with additional nil parameters' do
+      let(:params) { {id: '5', abc: nil} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc' }
+    end
+
+    context 'with additional false parameters' do
+      let(:params) { {id: '5', abc: false} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=false' }
+    end
+
+    context 'with additional true parameters' do
+      let(:params) { {id: '5', abc: true} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=true' }
+    end
+
     context 'with additional #to_param parameter' do
       let(:params) { {id: '5', abc: cls_to_param.new} }
 
       it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=42' }
     end
+
+    context 'with additional array parameter' do
+      let(:params) { {id: 5, abc: [1, 2]} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=1&abc=2' }
+    end
+
+    context 'with additional array parameter with objects' do
+      let(:params) { {id: 5, abc: [1, 2, cls_to_param.new]} }
+
+      it { expect(expanded.to_s).to eq 'http://test.host/resource/5?abc=1&abc=2&abc=42' }
+    end
+
+    context 'with additional nested array parameter' do
+      let(:params) { {id: 5, abc: [1, 2, [1]]} }
+
+      it { expect { expanded }.to raise_exception TypeError, /Can't convert Array into String./ }
+    end
+
+    context 'with additional hash parameter' do
+      let(:params) { {id: 5, abc: {a: 1, b: 2}} }
+
+      it { expect { expanded }.to raise_exception TypeError, /Can't convert Hash into String./ }
+    end
   end
 end