Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Loosen type check for Mongo::Collection#aggregate #174

Closed
wants to merge 2 commits into from

2 participants

@kamarcum

Mongo::Collection#aggregate was doing a strict arg.class == Array and arg.class == Hash. I changed this to use is_a? to make it easier to pass arguments like HashWithIndifferentAccess, which shouldn't interfere with the functionality.

A simple test is included.

Thanks!

@brandonblack

@kamarcum thanks for the contribution, but it looks like this broke a few tests in a few different versions of Ruby. Aside from the failures in Travis, make sure you've also validated your change with rake test:replica_set in 1.8.7, 1.9.3 and JRuby.

@kamarcum

Those two failures on Travis are pretty mysterious to me. Any hints?
I had only run rake test, and only with 1.9.3. I'll see if I can reproduce any of that locally.

@kamarcum

Well. I'm glad in a way that commit didn't make a difference in Travis. I couldn't think of a reason 1.8.7 would treat this code differently, so I guessed.

@brandonblack

Looks like a few of those initial failures were not related to your change and just Travis weirdness (specifically JRuby). However, it's definitely still failing for for Ruby 1.8.x. I'd love to merge this change, and I think it's a great contribution but making it work for 1.8.x is a must before I can do so.

Let me know when you've had a chance to look at this build failure:
https://travis-ci.org/mongodb/mongo-ruby-driver/jobs/5448949

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 15 additions and 2 deletions.
  1. +2 −2 lib/mongo/collection.rb
  2. +13 −0 test/functional/collection_test.rb
View
4 lib/mongo/collection.rb
@@ -651,8 +651,8 @@ def find_and_modify(opts={})
# @raise MongoOperationFailure if the aggregate command fails.
#
def aggregate(pipeline=nil, opts={})
- raise MongoArgumentError, "pipeline must be an array of operators" unless pipeline.class == Array
- raise MongoArgumentError, "pipeline operators must be hashes" unless pipeline.all? { |op| op.class == Hash }
+ raise MongoArgumentError, "pipeline must be an array of operators" unless pipeline.is_a? Array
+ raise MongoArgumentError, "pipeline operators must be hashes" unless pipeline.all? { |op| op.is_a? Hash }
hash = BSON::OrderedHash.new
hash['aggregate'] = self.name
View
13 test/functional/collection_test.rb
@@ -688,6 +688,19 @@ def test_aggregate_requires_valid_arguments
end
end
+ class MyHash < Hash; end
+ class MyArray < Array; end
+ def test_aggregate_accepts_class_extensions_in_args
+ setup_aggregate_data
+ my_hash = MyHash.new
+ my_hash["$project"] = {"tags" => 1, "pageViews" => 1}
+ my_hash["$match"] = {"pageViews" => 7}
+ my_array = Array.new
+ my_array << my_hash
+ results = @@test.aggregate(my_array)
+ assert_equal 1, results.length
+ end
+
def test_aggregate_pipeline_operator_format
assert_raise Mongo::OperationFailure do
@@test.aggregate([{"$project" => "_id"}])
Something went wrong with that request. Please try again.