RUBY-1825 Include system collections when listing collections

.evergreen/run-atlas-tests.sh

@@ -22,10 +22,24 @@ setup_ruby
 install_deps
 
 echo "Running specs"
-bundle exec rspec spec/atlas -fd
-test_status=$?
-echo "TEST STATUS"
-echo ${test_status}
+
+test_status=0
+for uri in ATLAS_REPLICA_SET_URI ATLAS_SHARDED_URI ATLAS_FREE_TIER_URI \
+  ATLAS_TLS11_URI ATLAS_TLS12_URI
+do
+  # ${!foo} syntax is bash specific:
+  # https://stackoverflow.com/questions/14049057/bash-expand-variable-in-a-variable
+  export ATLAS_URI="${!uri}"
+
+  bundle exec rspec spec/atlas -fd
+  this_test_status=$?
+  echo "TEST STATUS"
+  echo ${this_test_status}
+
+  if test $this_test_status != 0; then
+    test_status=$this_test_status
+  fi
+done
 
 kill_jruby
 

lib/mongo/operation/collections_info.rb

@@ -50,7 +50,7 @@ def execute(server)
       private
 
       def selector(server)
-        { :name => { '$not' => /system\.|\$/ } }
+        {}
       end
 
       def message(server)

lib/mongo/operation/list_collections/command.rb

@@ -31,9 +31,7 @@ class Command
         private
 
         def selector(server)
-          (spec[SELECTOR] || {}).merge(
-              listCollections: 1, filter: { name: { '$not' => /system\.|\$/ }}
-          )
+          (spec[SELECTOR] || {}).merge(listCollections: 1)
         end
 
         def message(server)

lib/mongo/operation/list_collections/op_msg.rb

@@ -28,8 +28,7 @@ class OpMsg < OpMsgBase
         private
 
         def selector(server)
-          (spec[SELECTOR] || {}).merge(
-              listCollections: 1, filter: { name: { '$not' => /system\.|\$/ }})
+          (spec[SELECTOR] || {}).merge(listCollections: 1)
         end
       end
     end

spec/atlas/atlas_connectivity_spec.rb

@@ -1,16 +1,16 @@
 require 'lite_spec_helper'
 
 describe 'Atlas connectivity' do
-  shared_examples 'connects to Atlas' do
-    let(:uri) { ENV[var] }
-    let(:client) { Mongo::Client.new(uri) }
+  let(:uri) { ENV['ATLAS_URI'] }
+  let(:client) { Mongo::Client.new(uri) }
 
-    before do
-      if uri.nil?
-        skip "#{var} not set in environment"
-      end
+  before do
+    if uri.nil?
+      skip "ATLAS_URI not set in environment"
     end
+  end
 
+  it 'connects to Atlas' do
     it 'runs ismaster successfully' do
       result = client.database.command(:ismaster => 1)
       expect(result.documents.first['ismaster']).to be true
@@ -21,34 +21,4 @@
       expect(result).to be_a(Array)
     end
   end
-
-  context 'Atlas replica set' do
-    let(:var) { 'ATLAS_REPLICA_SET_URI' }
-
-    it_behaves_like 'connects to Atlas'
-  end
-
-  context 'Atlas sharded cluster' do
-    let(:var) { 'ATLAS_SHARDED_URI' }
-
-    it_behaves_like 'connects to Atlas'
-  end
-
-  context 'Atlas free tier replica set' do
-    let(:var) { 'ATLAS_FREE_TIER_URI' }
-
-    it_behaves_like 'connects to Atlas'
-  end
-
-  context 'Atlas TLS 1.1 only replica set' do
-    let(:var) { 'ATLAS_TLS11_URI' }
-
-    it_behaves_like 'connects to Atlas'
-  end
-
-  context 'Atlas TLS 1.2 only replica set' do
-    let(:var) { 'ATLAS_TLS12_URI' }
-
-    it_behaves_like 'connects to Atlas'
-  end
 end

spec/atlas/operations_spec.rb

@@ -0,0 +1,25 @@
+require 'lite_spec_helper'
+
+describe 'Operations' do
+  let(:uri) { ENV['ATLAS_URI'] }
+  let(:client) { Mongo::Client.new(uri) }
+
+  before do
+    if uri.nil?
+      skip "ATLAS_URI not set in environment"
+    end
+  end
+
+  describe 'list_collections' do
+    # Atlas free tier proxy enforces restrictions on list_collections
+    # arguments. This tests verifies that list_collections works on Atlas
+
+    it 'works' do
+      # We are not allowed to mutate the database, therefore the list of
+      # collections would generally be empty.
+      expect do
+        client.database.list_collections
+      end.not_to raise_error
+    end
+  end
+end

spec/mongo/database_spec.rb

@@ -101,7 +101,7 @@
     end
 
     it 'does not include system collections' do
-      expect(database.collection_names).to_not include('system.indexes')
+      expect(database.collection_names).to_not include('system.version')
     end
 
     context 'when provided a session' do
@@ -136,7 +136,9 @@
       end
 
       it 'returns all collections' do
-        expect(database.collection_names(batch_size: 1).select { |c| c =~ /dalmatians/}.size).to eq(2)
+        collection_names = database.collection_names(batch_size: 1)
+        expect(collection_names).to include('0_dalmatians')
+        expect(collection_names).to include('1_dalmatians')
       end
 
     end
@@ -155,23 +157,33 @@
     end
 
     before do
-      database[:users].drop
-      database[:users].create
+      database[:acol].drop
+      database[:acol].create
     end
 
     context 'server 3.0+' do
       min_server_fcv '3.0'
 
       it 'returns a list of the collections info' do
-        expect(result).to include('users')
+        expect(result).to include('acol')
       end
     end
 
     context 'server 2.6' do
       max_server_fcv '2.6'
 
       it 'returns a list of the collections info' do
-        expect(result).to include("#{SpecConfig.instance.test_db}.users")
+        expect(result).to include("#{SpecConfig.instance.test_db}.acol")
+      end
+    end
+
+    context 'on admin database' do
+      let(:database) do
+        described_class.new(root_authorized_client, 'admin')
+      end
+
+      it 'includes system collections' do
+        expect(result.any? { |name| name =~ /(^|\.)system\./ }).to be true
       end
     end
   end
@@ -196,9 +208,17 @@
       it 'returns collection objects for each name' do
         expect(database.collections).to include(collection)
       end
+    end
+
+    context 'on admin database' do
+
+      let(:database) do
+        described_class.new(root_authorized_client, 'admin')
+      end
 
-      it 'does not include the system collections' do
-        expect(database.collections.collect(&:name).none? { |name| name =~ /system\.|\$/ }).to be(true)
+      it 'includes the system collections' do
+        collection_names = database.collections.map(&:name)
+        expect(collection_names).to include('system.version')
       end
     end
 

spec/support/spec_config.rb

@@ -3,6 +3,8 @@
 class SpecConfig
   include Singleton
 
+  # NB: constructor should not do I/O as SpecConfig may be used by tests
+  # only loading the lite spec helper. Do I/O eagerly in accessor methods.
   def initialize
     @uri_options = {}
     if ENV['MONGODB_URI']
@@ -41,41 +43,49 @@ def initialize
         @uri_tls_options[k] = v
       end
     end
+  end
+
+  attr_reader :uri_options, :connect_options
 
-    if @addresses.nil?
-      # Discover deployment topology
+  def addresses
+    @addresses ||= begin
       if @mongodb_uri
-        # TLS options need to be merged for evergreen due to
-        # https://github.com/10gen/mongo-orchestration/issues/268
-        client = Mongo::Client.new(@mongodb_uri.servers, Mongo::Options::Redacted.new(
-          server_selection_timeout: 5,
-        ).merge(@mongodb_uri.uri_options).merge(ssl_options))
-        @addresses = @mongodb_uri.servers
+        @mongodb_uri.servers
       else
         client = Mongo::Client.new(['localhost:27017'], server_selection_timeout: 5)
-        @addresses = client.cluster.servers_list.map do |server|
+        client.cluster.next_primary
+        client.cluster.servers_list.map do |server|
           server.address.to_s
         end
       end
-      client.cluster.next_primary
-      case client.cluster.topology.class.name
+    end
+  end
+
+  def connect_options
+    @connect_options ||= begin
+      # Discover deployment topology.
+      # TLS options need to be merged for evergreen due to
+      # https://github.com/10gen/mongo-orchestration/issues/268
+      client = Mongo::Client.new(addresses, Mongo::Options::Redacted.new(
+        server_selection_timeout: 5,
+      ).merge(ssl_options))
+      options = case client.cluster.topology.class.name
       when /Replica/
-        @connect_options = { connect: :replica_set, replica_set: client.cluster.topology.replica_set_name }
+        { connect: :replica_set, replica_set: client.cluster.topology.replica_set_name }
       when /Sharded/
-        @connect_options = { connect: :sharded }
+        { connect: :sharded }
       when /Single/
-        @connect_options = { connect: :direct }
+        { connect: :direct }
       when /Unknown/
         raise "Could not detect topology because the test client failed to connect to MongoDB deployment"
       else
         raise "Weird topology #{client.cluster.topology}"
       end
       client.close
+      options
     end
   end
 
-  attr_reader :uri_options, :addresses, :connect_options
-
   # Environment
 
   def ci?