Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MONGOID-5677 [Monkey Patch Removal] Remove Hash#to_criteria and Criteria#to_criteria. Add Criteria.from_hash #5717

Merged
merged 3 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions lib/mongoid/criteria.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,25 @@ class Criteria
include Clients::Sessions
include Options

Mongoid.deprecate(self, :for_js)
class << self
# Convert the given hash to a criteria. Will iterate over each keys in the
# hash which must correspond to method on a criteria object. The hash
# must also include a "klass" key.
#
# @example Convert the hash to a criteria.
# Criteria.from_hash({ klass: Band, where: { name: "Depeche Mode" })
#
# @param [ Hash ] hash The hash to convert.
#
# @return [ Criteria ] The criteria.
def from_hash(hash)
criteria = Criteria.new(hash.delete(:klass) || hash.delete('klass'))
hash.each_pair do |method, args|
criteria = criteria.__send__(method, args)
end
criteria
end
end

# Static array used to check with method missing - we only need to ever
# instantiate once.
Expand Down Expand Up @@ -250,16 +268,16 @@ def merge(other)
# @example Merge another criteria into this criteria.
# criteria.merge(Person.where(name: "bob"))
#
# @param [ Criteria ] other The criteria to merge in.
# @param [ Criteria | Hash ] other The criteria to merge in.
#
# @return [ Criteria ] The merged criteria.
def merge!(other)
criteria = other.to_criteria
selector.merge!(criteria.selector)
options.merge!(criteria.options)
self.documents = criteria.documents.dup unless criteria.documents.empty?
self.scoping_options = criteria.scoping_options
self.inclusions = (inclusions + criteria.inclusions).uniq
other = self.class.from_hash(other) if other.is_a?(Hash)
selector.merge!(other.selector)
options.merge!(other.options)
self.documents = other.documents.dup unless other.documents.empty?
self.scoping_options = other.scoping_options
self.inclusions = (inclusions + other.inclusions).uniq
self
end

Expand Down Expand Up @@ -352,9 +370,11 @@ def respond_to?(name, include_private = false)
# criteria.to_criteria
#
# @return [ Criteria ] self.
# @deprecated
def to_criteria
self
end
Mongoid.deprecate(self, :to_criteria)

# Convert the criteria to a proc.
#
Expand Down Expand Up @@ -452,6 +472,7 @@ def for_js(javascript, scope = {})
end
js_query(code)
end
Mongoid.deprecate(self, :for_js)

private

Expand Down
8 changes: 3 additions & 5 deletions lib/mongoid/extensions/hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,11 @@ def resizable?
# { klass: Band, where: { name: "Depeche Mode" }.to_criteria
#
# @return [ Criteria ] The criteria.
# @deprecated
def to_criteria
criteria = Criteria.new(delete(:klass) || delete("klass"))
each_pair do |method, args|
criteria = criteria.__send__(method, args)
end
criteria
Criteria.from_hash(self)
end
Mongoid.deprecate(self, :to_criteria)

private

Expand Down
137 changes: 91 additions & 46 deletions spec/mongoid/criteria_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1461,52 +1461,68 @@
end
end

describe "#merge!" do
describe '#merge!' do
let(:band) { Band.new }
let(:criteria) { Band.scoped.where(name: 'Depeche Mode').asc(:name) }
let(:association) { Band.relations['records'] }
subject(:merged) { criteria.merge!(other) }

let(:band) do
Band.new
end
context 'when merging a Criteria' do
let(:other) do
{ klass: Band, includes: [:records] }
end

let(:criteria) do
Band.scoped.where(name: "Depeche Mode").asc(:name)
end
it 'merges the selector' do
expect(merged.selector).to eq({ 'name' => 'Depeche Mode' })
end

let(:mergeable) do
Band.includes(:records).tap do |crit|
crit.documents = [ band ]
it 'merges the options' do
expect(merged.options).to eq({ sort: { 'name' => 1 }})
end
end

let(:association) do
Band.relations["records"]
end
it 'merges the scoping options' do
expect(merged.scoping_options).to eq([ nil, nil ])
end

let(:merged) do
criteria.merge!(mergeable)
end
it 'merges the inclusions' do
expect(merged.inclusions).to eq([ association ])
end

it "merges the selector" do
expect(merged.selector).to eq({ "name" => "Depeche Mode" })
it 'returns the same criteria' do
expect(merged).to equal(criteria)
end
end

it "merges the options" do
expect(merged.options).to eq({ sort: { "name" => 1 }})
end
context 'when merging a Hash' do
let(:other) do
Band.includes(:records).tap do |crit|
crit.documents = [ band ]
end
end

it "merges the documents" do
expect(merged.documents).to eq([ band ])
end
it 'merges the selector' do
expect(merged.selector).to eq({ 'name' => 'Depeche Mode' })
end

it "merges the scoping options" do
expect(merged.scoping_options).to eq([ nil, nil ])
end
it 'merges the options' do
expect(merged.options).to eq({ sort: { 'name' => 1 }})
end

it "merges the inclusions" do
expect(merged.inclusions).to eq([ association ])
end
it 'merges the documents' do
expect(merged.documents).to eq([ band ])
end

it 'merges the scoping options' do
expect(merged.scoping_options).to eq([ nil, nil ])
end

it "returns the same criteria" do
expect(merged).to equal(criteria)
it 'merges the inclusions' do
expect(merged.inclusions).to eq([ association ])
end

it 'returns the same criteria' do
expect(merged).to equal(criteria)
end
end
end

Expand Down Expand Up @@ -2308,17 +2324,6 @@ def self.ages; self; end
end
end

describe "#to_criteria" do

let(:criteria) do
Band.all
end

it "returns self" do
expect(criteria.to_criteria).to eq(criteria)
end
end

describe "#to_proc" do

let(:criteria) do
Expand Down Expand Up @@ -3031,11 +3036,11 @@ def self.ages; self; end
context "when the method exists on the criteria" do

before do
expect(criteria).to receive(:to_criteria).and_call_original
expect(criteria).to receive(:only).and_call_original
end

it "calls the method on the criteria" do
expect(criteria.to_criteria).to eq(criteria)
expect(criteria.only).to eq(criteria)
end
end

Expand Down Expand Up @@ -3242,4 +3247,44 @@ def self.ages; self; end
end
end
end

describe '.from_hash' do
subject(:criteria) { described_class.from_hash(hash) }

context 'when klass is specified' do
let(:hash) do
{ klass: Band, where: { name: 'Songs Ohia' } }
end

it 'returns a criteria' do
expect(criteria).to be_a(Mongoid::Criteria)
end

it 'sets the klass' do
expect(criteria.klass).to eq(Band)
end

it 'sets the selector' do
expect(criteria.selector).to eq({ 'name' => 'Songs Ohia' })
end
end

context 'when klass is missing' do
let(:hash) do
{ where: { name: 'Songs Ohia' } }
end

it 'returns a criteria' do
expect(criteria).to be_a(Mongoid::Criteria)
end

it 'has klass nil' do
expect(criteria.klass).to be_nil
end

it 'sets the selector' do
expect(criteria.selector).to eq({ 'name' => 'Songs Ohia' })
end
end
end
end