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-4914 Unmask Enumerable#find #4802

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
36 changes: 36 additions & 0 deletions lib/mongoid/criteria.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ module Mongoid
# against the database.
class Criteria
include Enumerable
alias :_enumerable_find :find
include Contextual
include Queryable
include Findable
alias :_findable_find :find
include Inspectable
include Includable
include Marshalable
Expand Down Expand Up @@ -55,6 +57,40 @@ def ==(other)
entries == other
end

# Call either Findable#find or Enumerable#find based on the given arguments
#
# If the arguments match Enumerable#find signature, that will be invoked.
# Otherwise Findable#find will be invoked
#
# The signature of Enumerable#find:
#
# find(ifnone = nil) { |obj| block } → obj or nil
# find(ifnone = nil) → an_enumerator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need to duplicate standard library documentation in our docstrings. Linking to stdlib docs I would say is sufficient and this way our documentation cannot get out of date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call - fixed at b58cf1d

#
# @example Find using a block, invokes Enumerable#find
# criteria.find { |item| item.name == "Depeche Mode" }
#
# @example Find given a block and a Proc, invokes Enumerable#find
# criteria.find(-> { "Default Band" }) { |item| item.name == "Milwaukee Mode" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there actual usage of this pattern in the wild, considering one can just as easily say

     #   criteria.find { |item| item.name == "Milwaukee Mode" } || 'Default Band'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've never seen it. I was just trying to cover all the cases for Enumerable#find :)

#
# @example Find given just a Proc, invokes Enumerable#find, returns an Enumerator
# enumerator = criteria.find(-> { "Default Band" })
#
# @example Find given anything else, invokes Findable#find
# critera.find("1234")
#
# @return Same return value as either Findable#find or Enumerable#find, depending
# on which one was invoked.
#
# @since 7.1.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer use since tags, and this one isn't accurate since the change would go into the next major release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed at 397f4c2

def find(*args, &block)
if block_given? || args.first&.is_a?(Proc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose changing this to if block_given? only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me :) Fixed at c08d43f

_enumerable_find(*args, &block)
else
_findable_find(*args)
end
end

# Needed to properly get a criteria back as json
#
# @example Get the criteria as json.
Expand Down
38 changes: 38 additions & 0 deletions spec/mongoid/criteria_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,44 @@
end
end

describe "#find" do
let!(:depeche) do
Band.create(name: "Depeche Mode")
end

let(:criteria) do
Band.where(name: "Depeche Mode")
end

context "when given a block" do
it "behaves as enumerable" do
result = criteria.find { |c| c.name == "Depeche Mode" }
expect(result).to eq(depeche)
end
end

context "when given a Proc and a block" do
it "behaves as enumerable" do
result = criteria.find(-> {"default"}) { |c| c.name == "Not Depeche Mode" }
expect(result).to eq("default")
end
end

context "when given a Proc" do
it "behaves as enumerable" do
result = criteria.find(-> {"default"})
expect(result).to be_a(Enumerator)
end
end

context "when given an id" do
it "behaves as findable" do
result = criteria.find(depeche.id)
expect(result).to eq(depeche)
end
end
end

describe "#find_one_and_update" do

let!(:depeche) do
Expand Down