[Fix toptal#795] Use array to store the value of Order parameter

To allow multiple sorting options that may have the same key name.
For example script based sorting whose key will always be `_script`.

CHANGELOG.md

@@ -8,6 +8,9 @@
 
 ### Bugs Fixed
 
+  * [#795](https://github.com/toptal/chewy/issues/795): Change the Chewy::Search::Parameters::Order implementation to use Array ([@jiajiawang][])
+    * To allow multiple sorting options that may have the same key name. For example script based sorting whose key will always be `_script`
+
 ## 7.2.1 (2021-05-11)
 
 ### New Features
@@ -631,6 +634,7 @@
 [@inbeom]: https://github.com/inbeom
 [@jesjos]: https://github.com/jesjos
 [@JF-Lalonde]: https://github.com/JF-Lalonde
+[@jiajiawang]: https://github.com/jiajiawang
 [@jimmybaker]: https://github.com/jimmybaker
 [@jirikolarik]: https://github.com/jirikolarik
 [@jirutka]: https://github.com/jirutka

lib/chewy/search/parameters/order.rb

@@ -17,7 +17,7 @@ class Order < Storage
         # @param other_value [Object] any acceptable storage value
         # @return [Object] updated value
         def update!(other_value)
-          value.merge!(normalize(other_value))
+          @value.concat(normalize(other_value))
         end
 
         # Size requires specialized rendering logic, it should return
@@ -28,34 +28,21 @@ def update!(other_value)
         def render
           return if value.blank?
 
-          sort = value.map do |(field, options)|
-            options ? {field => options} : field
-          end
-          {sort: sort}
-        end
-
-        # Comparison also reqires additional logic. Hashes are compared
-        # orderlessly, but for `sort` parameter oder is important, so we
-        # compare hash key collections additionally.
-        #
-        # @see Chewy::Search::Parameters::Storage#==
-        # @return [true, false]
-        def ==(other)
-          super && value.keys == other.value.keys
+          {sort: value}
         end
 
       private
 
         def normalize(value)
           case value
           when Array
-            value.each_with_object({}) do |sv, res|
-              res.merge!(normalize(sv))
+            value.each_with_object([]) do |sv, res|
+              res.concat(normalize(sv))
             end
           when Hash
-            value.stringify_keys
+            [value.stringify_keys]
           else
-            value.present? ? {value.to_s => nil} : {}
+            value.present? ? [value.to_s] : []
           end
         end
       end

spec/chewy/search/parameters/order_spec.rb

@@ -4,34 +4,37 @@
   subject { described_class.new(%i[foo bar]) }
 
   describe '#initialize' do
-    specify { expect(described_class.new.value).to eq({}) }
-    specify { expect(described_class.new(nil).value).to eq({}) }
-    specify { expect(described_class.new('').value).to eq({}) }
-    specify { expect(described_class.new(42).value).to eq('42' => nil) }
-    specify { expect(described_class.new([42, 43]).value).to eq('42' => nil, '43' => nil) }
-    specify { expect(described_class.new(a: 1).value).to eq('a' => 1) }
-    specify { expect(described_class.new(['', 43, {a: 1}]).value).to eq('a' => 1, '43' => nil) }
+    specify { expect(described_class.new.value).to eq([]) }
+    specify { expect(described_class.new(nil).value).to eq([]) }
+    specify { expect(described_class.new('').value).to eq([]) }
+    specify { expect(described_class.new(42).value).to eq(['42']) }
+    specify { expect(described_class.new([42, 43]).value).to eq(%w[42 43]) }
+    specify { expect(described_class.new([42, 42]).value).to eq(%w[42 42]) }
+    specify { expect(described_class.new([42, [43, 44]]).value).to eq(%w[42 43 44]) }
+    specify { expect(described_class.new(a: 1).value).to eq([{'a' => 1}]) }
+    specify { expect(described_class.new(['a', {a: 1}, {a: 2}]).value).to eq(['a', {'a' => 1}, {'a' => 2}]) }
+    specify { expect(described_class.new(['', 43, {a: 1}]).value).to eq(['43', {'a' => 1}]) }
   end
 
   describe '#replace!' do
     specify do
       expect { subject.replace!(foo: {}) }
         .to change { subject.value }
-        .from('foo' => nil, 'bar' => nil).to('foo' => {})
+        .from(%w[foo bar]).to([{'foo' => {}}])
     end
 
     specify do
       expect { subject.replace!(nil) }
         .to change { subject.value }
-        .from('foo' => nil, 'bar' => nil).to({})
+        .from(%w[foo bar]).to([])
     end
   end
 
   describe '#update!' do
     specify do
       expect { subject.update!(foo: {}) }
         .to change { subject.value }
-        .from('foo' => nil, 'bar' => nil).to('foo' => {}, 'bar' => nil)
+        .from(%w[foo bar]).to(['foo', 'bar', {'foo' => {}}])
     end
 
     specify { expect { subject.update!(nil) }.not_to change { subject.value } }
@@ -41,7 +44,7 @@
     specify do
       expect { subject.merge!(described_class.new(foo: {})) }
         .to change { subject.value }
-        .from('foo' => nil, 'bar' => nil).to('foo' => {}, 'bar' => nil)
+        .from(%w[foo bar]).to(['foo', 'bar', {'foo' => {}}])
     end
 
     specify { expect { subject.merge!(described_class.new) }.not_to change { subject.value } }
@@ -51,6 +54,7 @@
     specify { expect(described_class.new.render).to be_nil }
     specify { expect(described_class.new(:foo).render).to eq(sort: ['foo']) }
     specify { expect(described_class.new([:foo, {bar: 42}, :baz]).render).to eq(sort: ['foo', {'bar' => 42}, 'baz']) }
+    specify { expect(described_class.new([:foo, {bar: 42}, {bar: 43}, :baz]).render).to eq(sort: ['foo', {'bar' => 42}, {'bar' => 43}, 'baz']) }
   end
 
   describe '#==' do
@@ -59,7 +63,10 @@
     specify { expect(described_class.new(:foo)).not_to eq(described_class.new(:bar)) }
     specify { expect(described_class.new(%i[foo bar])).to eq(described_class.new(%i[foo bar])) }
     specify { expect(described_class.new(%i[foo bar])).not_to eq(described_class.new(%i[bar foo])) }
+    specify { expect(described_class.new(%i[foo foo])).not_to eq(described_class.new(%i[foo])) }
     specify { expect(described_class.new(foo: {a: 42})).to eq(described_class.new(foo: {a: 42})) }
     specify { expect(described_class.new(foo: {a: 42})).not_to eq(described_class.new(foo: {b: 42})) }
+    specify { expect(described_class.new(['foo', {'foo' => 42}])).not_to eq(described_class.new([{'foo' => 42}, 'foo'])) }
+    specify { expect(described_class.new([{'foo' => 42}, {'foo' => 43}])).not_to eq(described_class.new([{'foo' => 43}, {'foo' => 42}])) }
   end
 end

spec/chewy/search/parameters_spec.rb

@@ -13,7 +13,7 @@
 
     specify { expect(subject.storages[:limit]).to equal(limit) }
     specify { expect(subject.storages[:limit].value).to eq(3) }
-    specify { expect(subject.storages[:order].value).to eq('foo' => nil) }
+    specify { expect(subject.storages[:order].value).to eq(['foo']) }
 
     specify { expect { described_class.new(offset: limit) }.to raise_error(TypeError) }
   end

spec/chewy/search/request_spec.rb

@@ -177,7 +177,7 @@
   describe '#order' do
     specify { expect(subject.order(:foo).render[:body]).to include(sort: ['foo']) }
     specify { expect(subject.order(foo: 42).order(nil).render[:body]).to include(sort: ['foo' => 42]) }
-    specify { expect(subject.order(foo: 42).order(foo: 43).render[:body]).to include(sort: ['foo' => 43]) }
+    specify { expect(subject.order(foo: 42).order(foo: 43).render[:body]).to include(sort: [{'foo' => 42}, {'foo' => 43}]) }
     specify { expect(subject.order(:foo).order(:bar, :baz).render[:body]).to include(sort: %w[foo bar baz]) }
     specify { expect(subject.order(nil).render[:body]).to be_blank }
     specify { expect { subject.order(:foo) }.not_to change { subject.render } }