From 5f0bd100382aca63ab6e18573645726fbfbc68b8 Mon Sep 17 00:00:00 2001
From: johnnyshields <27655+johnnyshields@users.noreply.github.com>
Date: Sun, 3 Sep 2023 03:28:03 +0900
Subject: [PATCH 1/3] - Remove ``Object#blank_criteria?`` method entirely (was
previously deprecated and not used in code.) - Remove
``Hash#_mongoid_unsatisfiable_criteria?`` method is removed (was previously
marked @api private) and move it to a private method of
Referenced::HasMany::Enumerable.
---
docs/release-notes/mongoid-9.0.txt | 2 +
.../referenced/has_many/enumerable.rb | 39 ++++++-
lib/mongoid/extensions/array.rb | 22 ----
lib/mongoid/extensions/hash.rb | 50 ---------
lib/mongoid/extensions/object.rb | 16 ---
.../referenced/has_many/enumerable_spec.rb | 86 +++++++++++++++
spec/mongoid/extensions/array_spec.rb | 28 -----
spec/mongoid/extensions/hash_spec.rb | 103 ------------------
spec/mongoid/extensions/object_spec.rb | 7 --
9 files changed, 126 insertions(+), 227 deletions(-)
diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt
index 0a5ab664bd..1c598c328b 100644
--- a/docs/release-notes/mongoid-9.0.txt
+++ b/docs/release-notes/mongoid-9.0.txt
@@ -51,6 +51,8 @@ Deprecated functionality removed
The method ``Mongoid::QueryCache#clear_cache`` should be replaced with ``Mongo::QueryCache#clear``.
All other methods and submodules are identically named. Refer to the `driver query cache documentation
`_ for more details.
+- ``Object#blank_criteria?`` method is removed (was previously deprecated.)
+- ``Hash#_mongoid_unsatisfiable_criteria?`` method is removed (was previously marked @api private.)
``touch`` method now clears changed state
diff --git a/lib/mongoid/association/referenced/has_many/enumerable.rb b/lib/mongoid/association/referenced/has_many/enumerable.rb
index 131a095680..bd936f71cf 100644
--- a/lib/mongoid/association/referenced/has_many/enumerable.rb
+++ b/lib/mongoid/association/referenced/has_many/enumerable.rb
@@ -478,12 +478,49 @@ def set_base(document)
end
def unloaded_documents
- if _unloaded.selector._mongoid_unsatisfiable_criteria?
+ if unsatisfiable_criteria?(_unloaded.selector)
[]
else
_unloaded
end
end
+
+ # Checks whether conditions given in this hash are known to be
+ # unsatisfiable, i.e. querying with this hash will always return no
+ # documents.
+ #
+ # This method only handles condition shapes that Mongoid itself uses when
+ # it builds association queries. It does not guarantee that a false
+ # return value means the condition can produce a non-empty document set -
+ # only that if the return value is true, the condition always produces
+ # an empty document set.
+ #
+ # @example Unsatisfiable conditions
+ # unsatisfiable_criteria?({'_id' => {'$in' => []}})
+ # # => true
+ #
+ # @example Conditions which could be satisfiable
+ # unsatisfiable_criteria?({'_id' => '123'})
+ # # => false
+ #
+ # @example Conditions which are unsatisfiable that this method does not handle
+ # unsatisfiable_criteria?({'foo' => {'$in' => []}})
+ # # => false
+ #
+ # @param [ Hash ] selector The conditions to check.
+ #
+ # @return [ true | false ] Whether hash contains known unsatisfiable
+ # conditions.
+ def unsatisfiable_criteria?(selector)
+ unsatisfiable_criteria = { '_id' => { '$in' => [] } }
+ return true if selector == unsatisfiable_criteria
+ return false unless selector.length == 1 && selector.keys == %w[$and]
+
+ value = selector.values.first
+ value.is_a?(Array) && value.any? do |sub_value|
+ sub_value.is_a?(Hash) && unsatisfiable_criteria?(sub_value)
+ end
+ end
end
end
end
diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb
index 812ac620da..39988e361b 100644
--- a/lib/mongoid/extensions/array.rb
+++ b/lib/mongoid/extensions/array.rb
@@ -54,26 +54,6 @@ def __mongoize_time__
::Time.configured.local(*self)
end
- # Checks whether conditions given in this array are known to be
- # unsatisfiable, i.e., querying with this array will always return no
- # documents.
- #
- # This method used to assume that the array is the list of criteria
- # to be used with an $and operator. This assumption is no longer made;
- # therefore, since the interpretation of conditions in the array differs
- # between $and, $or and $nor operators, this method now always returns
- # false.
- #
- # This method is deprecated. Mongoid now uses
- # +_mongoid_unsatisfiable_criteria?+ internally; this method is retained
- # for backwards compatibility only. It always returns false.
- #
- # @return [ false ] Always false.
- # @deprecated
- def blank_criteria?
- false
- end
-
# Is the array a set of multiple arguments in a method?
#
# @example Is this multi args?
@@ -172,5 +152,3 @@ def resizable?
::Array.__send__(:include, Mongoid::Extensions::Array)
::Array.extend(Mongoid::Extensions::Array::ClassMethods)
-
-::Mongoid.deprecate(Array, :blank_criteria)
diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb
index 00104c38ab..cebf9923aa 100644
--- a/lib/mongoid/extensions/hash.rb
+++ b/lib/mongoid/extensions/hash.rb
@@ -58,54 +58,6 @@ def __consolidate__(klass)
consolidated
end
- # Checks whether conditions given in this hash are known to be
- # unsatisfiable, i.e., querying with this hash will always return no
- # documents.
- #
- # This method only handles condition shapes that Mongoid itself uses when
- # it builds association queries. It does not guarantee that a false
- # return value means the condition can produce a non-empty document set -
- # only that if the return value is true, the condition always produces
- # an empty document set.
- #
- # @example Unsatisfiable conditions
- # {'_id' => {'$in' => []}}._mongoid_unsatisfiable_criteria?
- # # => true
- #
- # @example Conditions which could be satisfiable
- # {'_id' => '123'}._mongoid_unsatisfiable_criteria?
- # # => false
- #
- # @example Conditions which are unsatisfiable that this method does not handle
- # {'foo' => {'$in' => []}}._mongoid_unsatisfiable_criteria?
- # # => false
- #
- # @return [ true | false ] Whether hash contains known unsatisfiable
- # conditions.
- # @api private
- def _mongoid_unsatisfiable_criteria?
- unsatisfiable_criteria = { "_id" => { "$in" => [] }}
- return true if self == unsatisfiable_criteria
- return false unless length == 1 && keys == %w($and)
- value = values.first
- value.is_a?(Array) && value.any? do |sub_v|
- sub_v.is_a?(Hash) && sub_v._mongoid_unsatisfiable_criteria?
- end
- end
-
- # Checks whether conditions given in this hash are known to be
- # unsatisfiable, i.e., querying with this hash will always return no
- # documents.
- #
- # This method is deprecated. Mongoid now uses
- # +_mongoid_unsatisfiable_criteria?+ internally; this method is retained
- # for backwards compatibility only.
- #
- # @return [ true | false ] Whether hash contains known unsatisfiable
- # conditions.
- # @deprecated
- alias :blank_criteria? :_mongoid_unsatisfiable_criteria?
-
# Deletes an id value from the hash.
#
# @example Delete an id value.
@@ -229,5 +181,3 @@ def resizable?
::Hash.__send__(:include, Mongoid::Extensions::Hash)
::Hash.extend(Mongoid::Extensions::Hash::ClassMethods)
-
-::Mongoid.deprecate(Hash, :blank_criteria)
diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb
index ec0cd89f91..47d7243fcd 100644
--- a/lib/mongoid/extensions/object.rb
+++ b/lib/mongoid/extensions/object.rb
@@ -74,20 +74,6 @@ def __to_inc__
self
end
- # Checks whether conditions given in this object are known to be
- # unsatisfiable, i.e., querying with this object will always return no
- # documents.
- #
- # This method is deprecated. Mongoid now uses
- # +_mongoid_unsatisfiable_criteria?+ internally; this method is retained
- # for backwards compatibility only. It always returns false.
- #
- # @return [ false ] Always false.
- # @deprecated
- def blank_criteria?
- false
- end
-
# Do or do not, there is no try. -- Yoda.
#
# @example Do or do not.
@@ -249,5 +235,3 @@ def mongoize(object)
::Object.__send__(:include, Mongoid::Extensions::Object)
::Object.extend(Mongoid::Extensions::Object::ClassMethods)
-
-::Mongoid.deprecate(Object, :blank_criteria)
diff --git a/spec/mongoid/association/referenced/has_many/enumerable_spec.rb b/spec/mongoid/association/referenced/has_many/enumerable_spec.rb
index 23d5cdd20a..2ec26fd4cb 100644
--- a/spec/mongoid/association/referenced/has_many/enumerable_spec.rb
+++ b/spec/mongoid/association/referenced/has_many/enumerable_spec.rb
@@ -2230,4 +2230,90 @@
end
end
end
+
+ describe "#unsatisfiable_criteria?" do
+ let(:criteria) { Person.all }
+ let(:enumerable) { described_class.new(criteria) }
+ subject { enumerable.send(:unsatisfiable_criteria?, selector) }
+
+ context 'when the selector has only an empty _id criteria' do
+ let(:selector) do
+ { '_id' => { '$in' => [] } }
+ end
+
+ it 'is true' do
+ expect(subject).to be true
+ end
+ end
+
+ context 'when the selector has an empty _id criteria and another criteria' do
+ let(:selector) do
+ { '_id' => { '$in' => [] }, 'foo' => 'bar' }
+ end
+
+ it 'is false' do
+ expect(subject).to be false
+ end
+ end
+
+ context 'when the selector has an empty _id criteria via $and' do
+ let(:selector) do
+ { '$and' => [{ '_id' => { '$in' => [] } }] }
+ end
+
+ it 'is true' do
+ expect(subject).to be true
+ end
+ end
+
+ context 'when the selector has an empty _id criteria via $and and another criteria at top level' do
+ let(:selector) do
+ { '$and' => [{ '_id' => { '$in' => [] } }], 'foo' => 'bar' }
+ end
+
+ it 'is false' do
+ expect(subject).to be false
+ end
+ end
+
+ context 'when the selector has an empty _id criteria via $and and another criteria in $and' do
+ let(:selector) do
+ { '$and' => [{ '_id' => { '$in' => [] } }, { 'foo' => 'bar' }] }
+ end
+
+ it 'is true' do
+ expect(subject).to be true
+ end
+ end
+
+ context 'when the selector has an empty _id criteria via $and and another criteria in $and value' do
+ let(:selector) do
+ { '$and' => [{ '_id' => { '$in' => [] }, 'foo' => 'bar' }] }
+ end
+
+ it 'is false' do
+ expect(subject).to be false
+ end
+ end
+
+ context 'when the selector has an empty _id criteria via $or' do
+ let(:selector) do
+ { '$or' => [{ '_id' => { '$in' => [] } }] }
+ end
+
+ it 'is false' do
+ expect(subject).to be false
+ end
+ end
+
+ context 'when the selector has an empty _id criteria via $nor' do
+ let(:selector) do
+ { '$nor' => [{ '_id' => { '$in' => [] } }] }
+ end
+
+ it 'is false' do
+ expect(subject).to be false
+ end
+ end
+ end
end
diff --git a/spec/mongoid/extensions/array_spec.rb b/spec/mongoid/extensions/array_spec.rb
index 18ab1ee2f2..886f23810e 100644
--- a/spec/mongoid/extensions/array_spec.rb
+++ b/spec/mongoid/extensions/array_spec.rb
@@ -378,34 +378,6 @@
end
end
- describe "#blank_criteria?" do
-
- context "when the array has an empty _id criteria" do
-
- context "when only the id criteria is in the array" do
-
- let(:array) do
- [{ "_id" => { "$in" => [] }}]
- end
-
- it "is false" do
- expect(array.blank_criteria?).to be false
- end
- end
-
- context "when the id criteria is in the array with others" do
-
- let(:array) do
- [{ "_id" => "test" }, { "_id" => { "$in" => [] }}]
- end
-
- it "is false" do
- expect(array.blank_criteria?).to be false
- end
- end
- end
- end
-
describe "#delete_one" do
context "when the object doesn't exist" do
diff --git a/spec/mongoid/extensions/hash_spec.rb b/spec/mongoid/extensions/hash_spec.rb
index cc4135a742..0bf4200360 100644
--- a/spec/mongoid/extensions/hash_spec.rb
+++ b/spec/mongoid/extensions/hash_spec.rb
@@ -351,107 +351,4 @@
expect(Hash).to be_resizable
end
end
-
- shared_examples_for 'unsatisfiable criteria method' do
-
- context "when the hash has only an empty _id criteria" do
-
- let(:hash) do
- { "_id" => { "$in" => [] }}
- end
-
- it "is true" do
- expect(hash.send(meth)).to be true
- end
- end
-
- context "when the hash has an empty _id criteria and another criteria" do
-
- let(:hash) do
- { "_id" => { "$in" => [] }, 'foo' => 'bar'}
- end
-
- it "is false" do
- expect(hash.send(meth)).to be false
- end
- end
-
- context "when the hash has an empty _id criteria via $and" do
-
- let(:hash) do
- {'$and' => [{ "_id" => { "$in" => [] }}]}
- end
-
- it "is true" do
- expect(hash.send(meth)).to be true
- end
- end
-
- context "when the hash has an empty _id criteria via $and and another criteria at top level" do
-
- let(:hash) do
- {'$and' => [{ "_id" => { "$in" => [] }}], 'foo' => 'bar'}
- end
-
- it "is false" do
- expect(hash.send(meth)).to be false
- end
- end
-
- context "when the hash has an empty _id criteria via $and and another criteria in $and" do
-
- let(:hash) do
- {'$and' => [{ "_id" => { "$in" => [] }}, {'foo' => 'bar'}]}
- end
-
- it "is true" do
- expect(hash.send(meth)).to be true
- end
- end
-
- context "when the hash has an empty _id criteria via $and and another criteria in $and value" do
-
- let(:hash) do
- {'$and' => [{ "_id" => { "$in" => [] }, 'foo' => 'bar'}]}
- end
-
- it "is false" do
- expect(hash.send(meth)).to be false
- end
- end
-
- context "when the hash has an empty _id criteria via $or" do
-
- let(:hash) do
- {'$or' => [{ "_id" => { "$in" => [] }}]}
- end
-
- it "is false" do
- expect(hash.send(meth)).to be false
- end
- end
-
- context "when the hash has an empty _id criteria via $nor" do
-
- let(:hash) do
- {'$nor' => [{ "_id" => { "$in" => [] }}]}
- end
-
- it "is false" do
- expect(hash.send(meth)).to be false
- end
- end
- end
-
- describe "#blank_criteria?" do
- let(:meth) { :blank_criteria? }
-
- it_behaves_like 'unsatisfiable criteria method'
- end
-
- describe "#_mongoid_unsatisfiable_criteria?" do
- let(:meth) { :_mongoid_unsatisfiable_criteria? }
-
- it_behaves_like 'unsatisfiable criteria method'
- end
end
diff --git a/spec/mongoid/extensions/object_spec.rb b/spec/mongoid/extensions/object_spec.rb
index 80d313a3af..551e3cb97e 100644
--- a/spec/mongoid/extensions/object_spec.rb
+++ b/spec/mongoid/extensions/object_spec.rb
@@ -279,11 +279,4 @@
expect(object.numeric?).to eq(false)
end
end
-
- describe "#blank_criteria?" do
-
- it "is false" do
- expect(object.blank_criteria?).to be false
- end
- end
end
From 7b6c78c40d8736bdb4f9de2a342cacbfefdad344 Mon Sep 17 00:00:00 2001
From: Johnny Shields <27655+johnnyshields@users.noreply.github.com>
Date: Sun, 3 Sep 2023 03:58:01 +0900
Subject: [PATCH 2/3] Update enumerable.rb
---
.../association/referenced/has_many/enumerable.rb | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/lib/mongoid/association/referenced/has_many/enumerable.rb b/lib/mongoid/association/referenced/has_many/enumerable.rb
index bd936f71cf..b388e404be 100644
--- a/lib/mongoid/association/referenced/has_many/enumerable.rb
+++ b/lib/mongoid/association/referenced/has_many/enumerable.rb
@@ -485,21 +485,20 @@ def unloaded_documents
end
end
- # Checks whether conditions given in this hash are known to be
+ # Checks whether conditions in the given hash are known to be
# unsatisfiable, i.e. querying with this hash will always return no
# documents.
#
# This method only handles condition shapes that Mongoid itself uses when
- # it builds association queries. It does not guarantee that a false
- # return value means the condition can produce a non-empty document set -
- # only that if the return value is true, the condition always produces
- # an empty document set.
+ # it builds association queries. Return value true indicates the condition
+ # always produces an empty document set. Note however that return value false
+ # is not a guarantee that the condition won't produce an empty document set.
#
# @example Unsatisfiable conditions
# unsatisfiable_criteria?({'_id' => {'$in' => []}})
# # => true
#
- # @example Conditions which could be satisfiable
+ # @example Conditions which may be satisfiable
# unsatisfiable_criteria?({'_id' => '123'})
# # => false
#
From 62e2ab09408485be40ef05b7731df574c145bfbb Mon Sep 17 00:00:00 2001
From: Jamis Buck
Date: Tue, 7 Nov 2023 15:09:36 -0700
Subject: [PATCH 3/3] removing the specs for unsatisifiable_criteria
this tests an implementation detail and not a behavior, which is
fragile. I'm not even sure this is an implementation detail we want,
and testing it specifically pours metaphorical concrete around it,
making it that much harder to remove later.
---
docs/release-notes/mongoid-9.0.txt | 1 -
.../referenced/has_many/enumerable_spec.rb | 86 -------------------
2 files changed, 87 deletions(-)
diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt
index db6dee7527..620f2a69a4 100644
--- a/docs/release-notes/mongoid-9.0.txt
+++ b/docs/release-notes/mongoid-9.0.txt
@@ -75,7 +75,6 @@ Deprecated functionality removed
All other methods and submodules are identically named. Refer to the `driver query cache documentation
`_ for more details.
- ``Object#blank_criteria?`` method is removed (was previously deprecated.)
-- ``Hash#_mongoid_unsatisfiable_criteria?`` method is removed (was previously marked @api private.)
``touch`` method now clears changed state
diff --git a/spec/mongoid/association/referenced/has_many/enumerable_spec.rb b/spec/mongoid/association/referenced/has_many/enumerable_spec.rb
index 2ec26fd4cb..23d5cdd20a 100644
--- a/spec/mongoid/association/referenced/has_many/enumerable_spec.rb
+++ b/spec/mongoid/association/referenced/has_many/enumerable_spec.rb
@@ -2230,90 +2230,4 @@
end
end
end
-
- describe "#unsatisfiable_criteria?" do
- let(:criteria) { Person.all }
- let(:enumerable) { described_class.new(criteria) }
- subject { enumerable.send(:unsatisfiable_criteria?, selector) }
-
- context 'when the selector has only an empty _id criteria' do
- let(:selector) do
- { '_id' => { '$in' => [] } }
- end
-
- it 'is true' do
- expect(subject).to be true
- end
- end
-
- context 'when the selector has an empty _id criteria and another criteria' do
- let(:selector) do
- { '_id' => { '$in' => [] }, 'foo' => 'bar' }
- end
-
- it 'is false' do
- expect(subject).to be false
- end
- end
-
- context 'when the selector has an empty _id criteria via $and' do
- let(:selector) do
- { '$and' => [{ '_id' => { '$in' => [] } }] }
- end
-
- it 'is true' do
- expect(subject).to be true
- end
- end
-
- context 'when the selector has an empty _id criteria via $and and another criteria at top level' do
- let(:selector) do
- { '$and' => [{ '_id' => { '$in' => [] } }], 'foo' => 'bar' }
- end
-
- it 'is false' do
- expect(subject).to be false
- end
- end
-
- context 'when the selector has an empty _id criteria via $and and another criteria in $and' do
- let(:selector) do
- { '$and' => [{ '_id' => { '$in' => [] } }, { 'foo' => 'bar' }] }
- end
-
- it 'is true' do
- expect(subject).to be true
- end
- end
-
- context 'when the selector has an empty _id criteria via $and and another criteria in $and value' do
- let(:selector) do
- { '$and' => [{ '_id' => { '$in' => [] }, 'foo' => 'bar' }] }
- end
-
- it 'is false' do
- expect(subject).to be false
- end
- end
-
- context 'when the selector has an empty _id criteria via $or' do
- let(:selector) do
- { '$or' => [{ '_id' => { '$in' => [] } }] }
- end
-
- it 'is false' do
- expect(subject).to be false
- end
- end
-
- context 'when the selector has an empty _id criteria via $nor' do
- let(:selector) do
- { '$nor' => [{ '_id' => { '$in' => [] } }] }
- end
-
- it 'is false' do
- expect(subject).to be false
- end
- end
- end
end