From 49055590400c72b2d5fb373c419684ae556fc458 Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 3 Sep 2023 04:49:03 +0900 Subject: [PATCH 1/5] Move Array#multi_arged? to Criteria::Findable.multiple_args? --- lib/mongoid/criteria/findable.rb | 14 ++++- lib/mongoid/extensions/array.rb | 10 ---- lib/mongoid/extensions/object.rb | 10 ---- spec/mongoid/criteria/findable_spec.rb | 68 ++++++++++++++++++++++++ spec/mongoid/extensions/array_spec.rb | 72 -------------------------- 5 files changed, 81 insertions(+), 93 deletions(-) diff --git a/lib/mongoid/criteria/findable.rb b/lib/mongoid/criteria/findable.rb index 09d557b44c..6df63dd11c 100644 --- a/lib/mongoid/criteria/findable.rb +++ b/lib/mongoid/criteria/findable.rb @@ -42,7 +42,7 @@ def execute_or_raise(ids, multi) def find(*args) ids = args.__find_args__ raise_invalid if ids.any?(&:nil?) - for_ids(ids).execute_or_raise(ids, args.multi_arged?) + for_ids(ids).execute_or_raise(ids, multiple_args?(args)) end # Adds a criterion to the +Criteria+ that specifies an id that must be matched. @@ -133,6 +133,18 @@ def mongoize_ids(ids) end end + # Is the given array a set of multiple arguments? + # + # @example Is this array multiple args? + # multiple_args?([ 1, 2, 3 ]) #=> true + # + # @param [ Array ] args The arguments. + # + # @return [ true | false ] If the array is multiple arguments. + def multiple_args?(args) + args.size > 1 || !args.first.is_a?(Hash) && args.first.resizable? + end + # Convenience method of raising an invalid options error. # # @example Raise the error. diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 812ac620da..430c97dc04 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -74,16 +74,6 @@ def blank_criteria? false end - # Is the array a set of multiple arguments in a method? - # - # @example Is this multi args? - # [ 1, 2, 3 ].multi_arged? - # - # @return [ true | false ] If the array is multi args. - def multi_arged? - !first.is_a?(Hash) && first.resizable? || size > 1 - end - # Turn the object from the ruby type we deal with to a Mongo friendly # type. # diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index ec0cd89f91..1c383ce764 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -130,16 +130,6 @@ def mongoize self end - # Is the object multi args. - # - # @example Is the object multi args? - # object.multi_arged? - # - # @return [ false ] false. - def multi_arged? - false - end - # Is the object a number? # # @example Is the object a number?. diff --git a/spec/mongoid/criteria/findable_spec.rb b/spec/mongoid/criteria/findable_spec.rb index 9715fe1ae3..71fa950fec 100644 --- a/spec/mongoid/criteria/findable_spec.rb +++ b/spec/mongoid/criteria/findable_spec.rb @@ -1130,4 +1130,72 @@ end end end + + describe "#multiple_args?" do + let(:criteria) { Mongoid::Criteria.new(Band) } + subject { criteria.send(:multiple_args?, args) } + + context "when there are multiple elements" do + let(:args) do + [ 1, 2, 3 ] + end + + it "returns true" do + expect(subject).to be true + end + end + + context "when there is one element" do + + context "when the element is a non enumerable" do + let(:args) do + [ 1 ] + end + + it "returns false" do + expect(subject).to be false + end + end + + context "when the element is resizable Hash instance" do + let(:args) do + [{'key' => 'value'}] + end + + it "returns false" do + expect(subject).to be false + end + end + + context "when the element is args of resizable Hash instances" do + let(:args) do + [[{'key1' => 'value2'},{'key1' => 'value2'}]] + end + + it "returns true" do + expect(subject).to be true + end + end + + context "when the element is an array" do + let(:args) do + [[ 1 ]] + end + + it "returns true" do + expect(subject).to be true + end + end + + context "when the element is a range" do + let(:args) do + [ 1..2 ] + end + + it "returns true" do + expect(subject).to be true + end + end + end + end end diff --git a/spec/mongoid/extensions/array_spec.rb b/spec/mongoid/extensions/array_spec.rb index 18ab1ee2f2..49c402986e 100644 --- a/spec/mongoid/extensions/array_spec.rb +++ b/spec/mongoid/extensions/array_spec.rb @@ -533,78 +533,6 @@ end end - describe "#multi_arged?" do - - context "when there are multiple elements" do - - let(:array) do - [ 1, 2, 3 ] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - - context "when there is one element" do - - context "when the element is a non enumerable" do - - let(:array) do - [ 1 ] - end - - it "returns false" do - expect(array).to_not be_multi_arged - end - end - - context "when the element is resizable Hash instance" do - - let(:array) do - [{'key' => 'value'}] - end - - it "returns false" do - expect(array).to_not be_multi_arged - end - end - - context "when the element is array of resizable Hash instances" do - - let(:array) do - [[{'key1' => 'value2'},{'key1' => 'value2'}]] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - - context "when the element is an array" do - - let(:array) do - [[ 1 ]] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - - context "when the element is a range" do - - let(:array) do - [ 1..2 ] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - end - end - describe ".resizable?" do it "returns true" do From 7aa9f124d5252a566e4bacd8b9c8b002fd5db331 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 3 Sep 2023 16:00:41 +0900 Subject: [PATCH 2/5] Update findable_spec.rb --- spec/mongoid/criteria/findable_spec.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/spec/mongoid/criteria/findable_spec.rb b/spec/mongoid/criteria/findable_spec.rb index 71fa950fec..2359fc1840 100644 --- a/spec/mongoid/criteria/findable_spec.rb +++ b/spec/mongoid/criteria/findable_spec.rb @@ -1169,7 +1169,7 @@ context "when the element is args of resizable Hash instances" do let(:args) do - [[{'key1' => 'value2'},{'key1' => 'value2'}]] + [[{ 'key1' => 'value2' }, { 'key1' => 'value2' }]] end it "returns true" do @@ -1177,7 +1177,7 @@ end end - context "when the element is an array" do + context "when the element is an Array" do let(:args) do [[ 1 ]] end @@ -1187,7 +1187,17 @@ end end - context "when the element is a range" do + context "when the element is a Set" do + let(:args) do + [Set[ 1 ]] + end + + it "returns true" do + expect(subject).to be true + end + end + + context "when the element is a Range" do let(:args) do [ 1..2 ] end From 3d12514a449dce89e81f432f9cc32876b9093e33 Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Mon, 4 Sep 2023 02:27:48 +0900 Subject: [PATCH 3/5] Clarify code docs --- lib/mongoid/criteria/findable.rb | 17 ++++++++++------- spec/mongoid/criteria/findable_spec.rb | 4 ++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/mongoid/criteria/findable.rb b/lib/mongoid/criteria/findable.rb index 6df63dd11c..d022603886 100644 --- a/lib/mongoid/criteria/findable.rb +++ b/lib/mongoid/criteria/findable.rb @@ -14,7 +14,8 @@ module Findable # criteria.execute_or_raise(id) # # @param [ Object ] ids The arguments passed. - # @param [ true | false ] multi Whether there arguments were a list. + # @param [ true | false ] multi Whether there arguments were a list, + # and therefore the return value should be an array. # # @raise [ Errors::DocumentNotFound ] If nothing returned. # @@ -42,7 +43,7 @@ def execute_or_raise(ids, multi) def find(*args) ids = args.__find_args__ raise_invalid if ids.any?(&:nil?) - for_ids(ids).execute_or_raise(ids, multiple_args?(args)) + for_ids(ids).execute_or_raise(ids, multi_args?(args)) end # Adds a criterion to the +Criteria+ that specifies an id that must be matched. @@ -133,15 +134,17 @@ def mongoize_ids(ids) end end - # Is the given array a set of multiple arguments? + # Indicates whether the given arguments array is a list of values. + # Used by the +find+ method to determine whether to return an array + # or single value. # - # @example Is this array multiple args? - # multiple_args?([ 1, 2, 3 ]) #=> true + # @example Are these arguments a list of values? + # multi_args?([ 1, 2, 3 ]) #=> true # # @param [ Array ] args The arguments. # - # @return [ true | false ] If the array is multiple arguments. - def multiple_args?(args) + # @return [ true | false ] Whether the arguments are a list. + def multi_args?(args) args.size > 1 || !args.first.is_a?(Hash) && args.first.resizable? end diff --git a/spec/mongoid/criteria/findable_spec.rb b/spec/mongoid/criteria/findable_spec.rb index 71fa950fec..5e269cbeba 100644 --- a/spec/mongoid/criteria/findable_spec.rb +++ b/spec/mongoid/criteria/findable_spec.rb @@ -1131,9 +1131,9 @@ end end - describe "#multiple_args?" do + describe "#multi_args?" do let(:criteria) { Mongoid::Criteria.new(Band) } - subject { criteria.send(:multiple_args?, args) } + subject { criteria.send(:multi_args?, args) } context "when there are multiple elements" do let(:args) do From cd8e34e2bc67896584e1ab7680e42b64fde8f90f Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Mon, 4 Sep 2023 02:42:05 +0900 Subject: [PATCH 4/5] Add pending for Set spec --- spec/mongoid/criteria/findable_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/mongoid/criteria/findable_spec.rb b/spec/mongoid/criteria/findable_spec.rb index eaa2cddf9b..fa2858031f 100644 --- a/spec/mongoid/criteria/findable_spec.rb +++ b/spec/mongoid/criteria/findable_spec.rb @@ -1193,6 +1193,8 @@ end it "returns true" do + pending 'https://jira.mongodb.org/browse/MONGOID-5666' + expect(subject).to be true end end From 647f86cf7d42ad60dcafeac3481025661518d6be Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Mon, 6 Nov 2023 15:46:18 -0700 Subject: [PATCH 5/5] deprecate __multi_arged?__ instead of removing it --- lib/mongoid/criteria/findable.rb | 2 +- lib/mongoid/extensions/array.rb | 13 ++++- lib/mongoid/extensions/object.rb | 12 ++++ spec/mongoid/criteria/findable_spec.rb | 80 -------------------------- 4 files changed, 25 insertions(+), 82 deletions(-) diff --git a/lib/mongoid/criteria/findable.rb b/lib/mongoid/criteria/findable.rb index d022603886..2f116dc353 100644 --- a/lib/mongoid/criteria/findable.rb +++ b/lib/mongoid/criteria/findable.rb @@ -109,7 +109,7 @@ def from_database(ids) from_database_selector(ids).entries end - private def from_database_selector(ids) + def from_database_selector(ids) if ids.size > 1 any_in(_id: ids) else diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 430c97dc04..7cf774c754 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -3,7 +3,6 @@ module Mongoid module Extensions - # Adds type-casting behavior to Array class. module Array @@ -74,6 +73,18 @@ def blank_criteria? false end + # Is the array a set of multiple arguments in a method? + # + # @example Is this multi args? + # [ 1, 2, 3 ].multi_arged? + # + # @return [ true | false ] If the array is multi args. + # @deprecated + def multi_arged? + !first.is_a?(Hash) && first.resizable? || size > 1 + end + Mongoid.deprecate(self, :multi_arged?) + # Turn the object from the ruby type we deal with to a Mongo friendly # type. # diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index 1c383ce764..a63df40c13 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -130,6 +130,18 @@ def mongoize self end + # Is the object multi args. + # + # @example Is the object multi args? + # object.multi_arged? + # + # @return [ false ] false. + # @deprecated + def multi_arged? + false + end + Mongoid.deprecate(self, :multi_arged?) + # Is the object a number? # # @example Is the object a number?. diff --git a/spec/mongoid/criteria/findable_spec.rb b/spec/mongoid/criteria/findable_spec.rb index fa2858031f..9715fe1ae3 100644 --- a/spec/mongoid/criteria/findable_spec.rb +++ b/spec/mongoid/criteria/findable_spec.rb @@ -1130,84 +1130,4 @@ end end end - - describe "#multi_args?" do - let(:criteria) { Mongoid::Criteria.new(Band) } - subject { criteria.send(:multi_args?, args) } - - context "when there are multiple elements" do - let(:args) do - [ 1, 2, 3 ] - end - - it "returns true" do - expect(subject).to be true - end - end - - context "when there is one element" do - - context "when the element is a non enumerable" do - let(:args) do - [ 1 ] - end - - it "returns false" do - expect(subject).to be false - end - end - - context "when the element is resizable Hash instance" do - let(:args) do - [{'key' => 'value'}] - end - - it "returns false" do - expect(subject).to be false - end - end - - context "when the element is args of resizable Hash instances" do - let(:args) do - [[{ 'key1' => 'value2' }, { 'key1' => 'value2' }]] - end - - it "returns true" do - expect(subject).to be true - end - end - - context "when the element is an Array" do - let(:args) do - [[ 1 ]] - end - - it "returns true" do - expect(subject).to be true - end - end - - context "when the element is a Set" do - let(:args) do - [Set[ 1 ]] - end - - it "returns true" do - pending 'https://jira.mongodb.org/browse/MONGOID-5666' - - expect(subject).to be true - end - end - - context "when the element is a Range" do - let(:args) do - [ 1..2 ] - end - - it "returns true" do - expect(subject).to be true - end - end - end - end end