re-require shoulda; gridfs decoupling

README.rdoc

@@ -292,10 +292,11 @@ It's also possible to test replica pairs with connection pooling:
 
   $ rake test:pooled_pair_insert
 
-===Mocha
+===Shoulda and Mocha
 
-Running the test suite requires mocha.  You can install it as follows:
+Running the test suite requires shoulda and mocha.  You can install them as follows:
 
+  $ gem install shoulda
   $ gem install mocha
 
 The tests assume that the Mongo database is running on the default port. You

lib/mongo/gridfs/grid.rb

@@ -22,22 +22,25 @@ class Grid
 
     def initialize(db, fs_name=DEFAULT_FS_NAME)
       check_params(db)
-      @db     = db
-      @files  = @db["#{fs_name}.files"]
-      @chunks = @db["#{fs_name}.chunks"]
+      @db      = db
+      @files   = @db["#{fs_name}.files"]
+      @chunks  = @db["#{fs_name}.chunks"]
+      @fs_name = fs_name
 
       @chunks.create_index([['files_id', Mongo::ASCENDING], ['n', Mongo::ASCENDING]])
     end
 
     def put(data, filename, opts={})
-      file = GridIO.new(@files, @chunks, filename, 'w', false, opts=opts)
+      opts.merge!(default_grid_io_opts)
+      file = GridIO.new(@files, @chunks, filename, 'w', opts=opts)
       file.write(data)
       file.close
       file.files_id
     end
 
     def get(id)
-      GridIO.new(@files, @chunks, nil, 'r', false, :_id => id)
+      opts = {:query => {'_id' => id}}.merge!(default_grid_io_opts)
+      GridIO.new(@files, @chunks, nil, 'r', opts)
     end
 
     def delete(id)
@@ -47,6 +50,10 @@ def delete(id)
 
     private
 
+    def default_grid_io_opts
+      {:fs_name => @fs_name}
+    end
+
     def check_params(db)
       if !db.is_a?(Mongo::DB)
         raise MongoArgumentError, "db must be an instance of Mongo::DB."

lib/mongo/gridfs/grid_file_system.rb

@@ -23,10 +23,12 @@ def initialize(db, fs_name=DEFAULT_FS_NAME)
       super
 
       @files.create_index([['filename', 1], ['uploadDate', -1]])
+      @default_query_opts = {:sort => [['filename', 1], ['uploadDate', -1]], :limit => 1}
     end
 
     def open(filename, mode, opts={})
-      file   = GridIO.new(@files, @chunks, filename, mode, true, opts)
+      opts.merge!(default_grid_io_opts(filename))
+      file   = GridIO.new(@files, @chunks, filename, mode, opts)
       return file unless block_given?
       result = nil
       begin
@@ -37,15 +39,31 @@ def open(filename, mode, opts={})
       result
     end
 
-    def put(data, filename)
+    def put(data, filename, opts={})
+      opts.merge!(default_grid_io_opts(filename))
+      file = GridIO.new(@files, @chunks, filename, 'w', opts)
+      file.write(data)
+      file.close
+      file.files_id
     end
 
-    def get(id)
+    def get(filename, opts={})
+      opts.merge!(default_grid_io_opts(filename))
+      GridIO.new(@files, @chunks, filename, 'r', opts)
     end
 
-    # Deletes all files matching the given criteria.
-    def delete(criteria)
+    def delete(filename, opts={})
+      ids = @files.find({'filename' => filename}, ['_id'])
+      ids.each do |id|
+        @files.remove({'_id' => id})
+        @chunks.remove('files_id' => id)
+      end
     end
 
+    private
+
+    def default_grid_io_opts(filename=nil)
+      {:fs_name => @fs_name, :query => {'filename' => filename}, :query_opts => @default_query_opts}
+    end
   end
 end

lib/mongo/gridfs/grid_io.rb

@@ -23,20 +23,20 @@ class GridIO
 
     attr_reader :content_type, :chunk_size, :upload_date, :files_id, :filename, :metadata
 
-    def initialize(files, chunks, filename, mode, filesystem, opts={})
-      @files    = files
-      @chunks   = chunks
-      @filename = filename
-      @mode     = mode
-      @content_type = opts[:content_type] || DEFAULT_CONTENT_TYPE
-      @chunk_size   = opts[:chunk_size] || DEFAULT_CHUNK_SIZE
-      @files_id     = opts[:_id]
+    def initialize(files, chunks, filename, mode, opts={})
+      @files      = files
+      @chunks     = chunks
+      @filename   = filename
+      @mode       = mode
+      @query      = opts[:query] || {}
+      @query_opts = opts[:query_opts] || {}
+      @fs_name    = opts[:fs_name] || Grid::DEFAULT_FS_NAME
 
       case @mode
-        when 'r' then init_read(filesystem, opts)
+        when 'r' then init_read(opts)
         when 'w' then init_write(opts)
         else
-          raise GridError, "Invalid file mode #{@mode}. Valid options include 'r' and 'w'."
+          raise GridError, "Invalid file mode #{@mode}. Mode should be 'r' or 'w'."
       end
     end
 
@@ -147,9 +147,7 @@ def create_chunk(n)
       chunk
     end
 
-    # TODO: Perhaps use an upsert here instead?
     def save_chunk(chunk)
-      @chunks.remove('_id' => chunk['_id'])
       @chunks.insert(chunk)
     end
 
@@ -159,22 +157,17 @@ def get_chunk(n)
       chunk
     end
 
-    def get_chunk_for_read(n)
-      chunk = get_chunk(n)
-      return nil unless chunk
-    end
-
     def last_chunk_number
       (@file_length / @chunk_size).to_i
     end
 
-    # Read a file in its entirety (optimized).
+    # Read a file in its entirety.
     def read_all
       buf = ''
       while true
         buf << @current_chunk['data'].to_s
-        break if @current_chunk['n'] == last_chunk_number
         @current_chunk = get_chunk(@current_chunk['n'] + 1)
+        break unless @current_chunk
       end
       buf
     end
@@ -232,15 +225,10 @@ def write_string(string)
       string.length - to_write
     end
 
-    # Initialize based on whether the supplied file exists.
-    def init_read(filesystem, opts)
-      if filesystem
-        doc = @files.find({'filename' => @filename}, :sort => [["uploadDate", -1]], :limit => 1).next_document
-        raise GridError, "Could not open file with filename #{@filename}" unless doc
-      else
-        doc = @files.find({'_id' => @files_id}).next_document
-        raise GridError, "Could not open file with id #{@files_id}" unless doc
-      end
+    # Initialize the class for reading a file.
+    def init_read(opts)
+      doc = @files.find(@query, @query_opts).next_document
+      raise GridError, "Could not open file matching #{@query.inspect} #{@query_opts.inspect}" unless doc
 
       @files_id     = doc['_id']
       @content_type = doc['contentType']
@@ -251,11 +239,12 @@ def init_read(filesystem, opts)
       @metadata     = doc['metadata']
       @md5          = doc['md5']
       @filename     = doc['filename']
+
       @current_chunk = get_chunk(0)
       @file_position = 0
     end
 
-    # Validates and sets up the class for the given file mode.
+    # Initialize the class for writing a file.
     def init_write(opts)
       @files_id      = opts[:_id] || Mongo::ObjectID.new
       @content_type  = opts[:content_type] || @content_type || DEFAULT_CONTENT_TYPE
@@ -281,7 +270,7 @@ def to_mongo_object
       # Get a server-side md5.
       md5_command            = OrderedHash.new
       md5_command['filemd5'] = @files_id
-      md5_command['root']    = 'fs'
+      md5_command['root']    = @fs_name
       h['md5']               = @files.db.command(md5_command)['md5']
 
       h

test/binary_test.rb

@@ -1,13 +1,15 @@
 # encoding:utf-8
 require 'test/test_helper'
 
-context "Inspecting" do
-  setup do
-    @data = ("THIS IS BINARY " * 50).unpack("c*")
-  end
+class BinaryTest < Test::Unit::TestCase
+  context "Inspecting" do
+    setup do
+      @data = ("THIS IS BINARY " * 50).unpack("c*")
+    end
 
-  should "not display actual data" do
-    binary = Mongo::Binary.new(@data)
-    assert_equal "<Mongo::Binary:#{binary.object_id}>", binary.inspect
+    should "not display actual data" do
+      binary = Mongo::Binary.new(@data)
+      assert_equal "<Mongo::Binary:#{binary.object_id}>", binary.inspect
+    end
   end
 end

test/collection_test.rb

@@ -1,7 +1,7 @@
 require 'test/test_helper'
 
 class TestCollection < Test::Unit::TestCase
-  @@connection = Connection.new(ENV['MONGO_RUBY_DRIVER_HOST'] || 'localhost', ENV['MONGO_RUBY_DRIVER_PORT'] || Connection::DEFAULT_PORT)
+  @@connection ||= Connection.new(ENV['MONGO_RUBY_DRIVER_HOST'] || 'localhost', ENV['MONGO_RUBY_DRIVER_PORT'] || Connection::DEFAULT_PORT)
   @@db   = @@connection.db('ruby-mongo-test')
   @@test = @@db.collection("test")
   @@version = @@connection.server_version
@@ -75,32 +75,31 @@ def test_nil_id
   end
 
   if @@version > "1.1"
-    context "distinct queries" do
-      setup do
-        @@test.remove
-        @@test.insert([{:a => 0, :b => {:c => "a"}},
-                       {:a => 1, :b => {:c => "b"}},
-                       {:a => 1, :b => {:c => "c"}},
-                       {:a => 2, :b => {:c => "a"}},
-                       {:a => 3},
-                       {:a => 3}])
-      end
-
-      should "return distinct values" do
-        assert_equal [0, 1, 2, 3], @@test.distinct(:a).sort
-        assert_equal ["a", "b", "c"], @@test.distinct("b.c").sort
-      end
-
-      if @@version >= "1.2"
+    def setup_for_distinct
+      @@test.remove
+      @@test.insert([{:a => 0, :b => {:c => "a"}},
+                     {:a => 1, :b => {:c => "b"}},
+                     {:a => 1, :b => {:c => "c"}},
+                     {:a => 2, :b => {:c => "a"}},
+                     {:a => 3},
+                     {:a => 3}])
+    end
 
-        should "filter collection with query" do
-          assert_equal [2, 3], @@test.distinct(:a, {:a => {"$gt" => 1}}).sort
-        end
+    def test_distinct_queries
+      setup_for_distinct
+      assert_equal [0, 1, 2, 3], @@test.distinct(:a).sort
+      assert_equal ["a", "b", "c"], @@test.distinct("b.c").sort
+    end
 
-        should "filter nested objects" do
-          assert_equal ["a", "b"], @@test.distinct("b.c", {"b.c" => {"$ne" => "c"}}).sort
-        end
+    if @@version >= "1.2"
+      def test_filter_collection_with_query
+        setup_for_distinct
+        assert_equal [2, 3], @@test.distinct(:a, {:a => {"$gt" => 1}}).sort
+      end
 
+      def test_filter_nested_objects
+        setup_for_distinct
+        assert_equal ["a", "b"], @@test.distinct("b.c", {"b.c" => {"$ne" => "c"}}).sort
       end
     end
   end