Merge pull request #220 from samlown/thread-safe-db

Upgrading to Couchrest 2.0.1 and simplifying connection multithreading

Author: samlown

couchrest_model.gemspec

@@ -22,13 +22,13 @@ Gem::Specification.new do |s|
   s.executables   = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) }
   s.require_paths = ["lib"]
 
-  s.add_dependency("couchrest",   "2.0.0")
+  s.add_dependency("couchrest",   "2.0.1")
   s.add_dependency("activemodel", ">= 4.0.2")
   s.add_dependency("tzinfo",      ">= 0.3.22")
   s.add_dependency("hashdiff",    "~> 0.3")
   s.add_development_dependency("rspec", "~> 3.5.0")
   s.add_development_dependency("rack-test", ">= 0.5.7")
-  s.add_development_dependency("rake", ">= 0.8.0")
+  s.add_development_dependency("rake", ">= 0.8.0", "< 11.0")
   s.add_development_dependency("test-unit")
   s.add_development_dependency("minitest", "> 4.1") #, "< 5.0") # For Kaminari and activesupport, pending removal
   s.add_development_dependency("kaminari", ">= 0.14.1", "< 0.16.0")

history.md

@@ -8,6 +8,8 @@
   * Migration improvements and tests ([PR](https://github.com/couchrest/couchrest_model/pull/215) @ellneal)
   * Add some guards to prevent view errors when using custom emit values ([PR](https://github.com/couchrest/couchrest_model/pull/214) @ellneal)
   * Making running the tests a bit easier using Docker ([PR](https://github.com/couchrest/couchrest_model/pull/213) @ellneal)
+  * Upgraded to Couchrest 2.0.1 ([PR](https://github.com/couchrest/couchrest_model/pull/220) @samlown)
+  * Removed :persistent and simplifying connection handling with aim to reduce number of connections and improve multithreading ([PR](https://github.com/couchrest/couchrest_model/pull/220) @samlown)
 
 ## 2.2.0.beta1 - 2016-08-20
 

lib/couchrest/model/connection.rb

@@ -9,40 +9,42 @@ def server
 
       module ClassMethods
 
-        # Overwrite the normal use_database method so that a database
+        # Overwrite the CouchRest::Document.use_database method so that a database
         # name can be provided instead of a full connection.
-        # The actual database will be validated when it is requested for use.
-        # Note that this should not be used with proxied models!
+        # We prepare the database immediatly, so ensure any connection details
+        # are provided in advance.
+        # Note that this will not work correctly with proxied models.
         def use_database(db)
-          @_use_database = db
+          @database = prepare_database(db)
         end
 
         # Overwrite the default database method so that it always
         # provides something from the configuration.
         # It will try to inherit the database from an ancester
-        # unless the use_database method has been used, in which
-        # case a new connection will be started.
+        # unless the use_database method has been used.
         def database
           @database ||= prepare_database(super)
         end
 
         def server
-          @server ||= CouchRest::Server.new(prepare_server_uri, :persistent => connection_configuration[:persistent])
+          @server ||= ServerPool.instance[prepare_server_uri]
         end
 
-        def prepare_database(db = nil, override_use_database = false)
-          db = @_use_database unless override_use_database || @_use_database.nil?
+        def prepare_database(db = nil)
           if db.nil? || db.is_a?(String) || db.is_a?(Symbol)
-            conf = connection_configuration
-            db = [conf[:prefix], db.to_s, conf[:suffix]].reject{|s| s.to_s.empty?}.join(conf[:join])
-            self.server.database!(db)
+            self.server.database!(prepare_database_name(db))
           else
             db
           end
         end
 
         protected
 
+        def prepare_database_name(base)
+          conf = connection_configuration
+          [conf[:prefix], base.to_s, conf[:suffix]].reject{|s| s.to_s.empty?}.join(conf[:join])
+        end
+
         def prepare_server_uri
           conf = connection_configuration
           userinfo = [conf[:username], conf[:password]].compact.join(':')
@@ -52,21 +54,14 @@ def prepare_server_uri
 
         def connection_configuration
           @connection_configuration ||=
-            self.connection.update(
+            self.connection.merge(
               (load_connection_config_file[environment.to_sym] || {}).symbolize_keys
             )
         end
 
         def load_connection_config_file
           file = connection_config_file
-          connection_config_cache[file] ||=
-            (File.exists?(file) ?
-              YAML::load(ERB.new(IO.read(file)).result) :
-              { }).symbolize_keys
-        end
-
-        def connection_config_cache
-          Thread.current[:connection_config_cache] ||= {}
+          ConnectionConfig.instance[file]
         end
 
       end

lib/couchrest/model/connection_config.rb

@@ -0,0 +1,32 @@
+module CouchRest
+  module Model
+  
+    # Thead safe caching of connection configuration files.
+    class ConnectionConfig
+      include Singleton
+
+      def initialize
+        @config_files = {}
+        @mutex = Mutex.new
+      end
+
+      def [](file)
+        @mutex.synchronize do
+          @config_files[file] ||= load_config(file)
+        end
+      end
+
+      private
+
+      def load_config(file)
+        if File.exists?(file)
+          YAML::load(ERB.new(IO.read(file)).result).symbolize_keys
+        else
+          { }
+        end
+      end
+
+    end
+
+  end
+end

lib/couchrest/model/proxyable.rb

@@ -8,8 +8,10 @@ def proxy_database(assoc_name)
         raise StandardError, "Please set the #proxy_database_method" if self.class.proxy_database_method.nil?
         db_name = self.send(self.class.proxy_database_method)
         db_suffix = self.class.proxy_database_suffixes[assoc_name.to_sym]
-        @proxy_databases ||= {}
-        @proxy_databases[assoc_name.to_sym] ||= self.class.prepare_database([db_name, db_suffix].compact.reject(&:blank?).join(self.class.connection[:join]), true)
+        @_proxy_databases ||= {}
+        @_proxy_databases[assoc_name.to_sym] ||= begin
+          self.class.prepare_database([db_name, db_suffix].compact.reject(&:blank?).join(self.class.connection[:join]))
+        end
       end
 
       module ClassMethods

lib/couchrest/model/server_pool.rb

@@ -0,0 +1,22 @@
+module CouchRest
+  module Model
+
+    # Simple Server Pool with thread safety so that a single server
+    # instance can be shared with multiple classes.
+    class ServerPool
+      include Singleton
+
+      def initialize
+        @servers = {}
+        @mutex = Mutex.new
+      end
+
+      def [](url)
+        @mutex.synchronize do
+          @servers[url] ||= CouchRest::Server.new(url)
+        end
+      end
+
+    end
+  end
+end

lib/couchrest_model.rb

@@ -26,6 +26,8 @@
 
 require "couchrest/model"
 require "couchrest/model/errors"
+require "couchrest/model/server_pool"
+require "couchrest/model/connection_config"
 require "couchrest/model/configuration"
 require "couchrest/model/translation"
 require "couchrest/model/persistence"

spec/unit/connection_config_spec.rb

@@ -0,0 +1,41 @@
+
+require "spec_helper"
+
+describe CouchRest::Model::ConnectionConfig do
+
+  subject { CouchRest::Model::ConnectionConfig }
+
+  describe ".instance" do
+
+    it "should provide a singleton" do 
+      expect(subject.instance).to be_a(CouchRest::Model::ConnectionConfig)
+    end
+
+  end
+
+  describe "#[file]" do
+
+    let :file do
+      File.join(FIXTURE_PATH, "config", "couchdb.yml")
+    end
+
+    it "should provide a config file hash" do
+      conf = subject.instance[file]
+      expect(conf).to be_a(Hash)
+    end
+
+    it "should provide a config file hash with symbolized keys" do
+      conf = subject.instance[file]
+      expect(conf[:development]).to be_a(Hash)
+      expect(conf[:development]['host']).to be_a(String)
+    end
+
+    it "should always provide same hash" do
+      f1 = subject.instance[file]
+      f2 = subject.instance[file]
+      expect(f1.object_id).to eql(f2.object_id)
+    end
+
+  end
+
+end

spec/unit/connection_spec.rb

@@ -58,12 +58,6 @@
         @class.use_database(db)
         expect(@class.database).to eql(db)
       end
-      it "should never prepare the database before it is needed" do
-        db = @class.server.database('test')
-        expect(@class).not_to receive(:prepare_database)
-        @class.use_database('test')
-        @class.use_database(db)
-      end
       it "should use the database specified" do
         @class.use_database(:test)
         expect(@class.database.name).to eql('couchrest_test')
@@ -131,15 +125,10 @@
 
       it "should use the .use_database value" do
         @class.use_database('testing')
-        db = @class.prepare_database
+        db = @class.database
         expect(db.name).to eql('couchrest_testing')
       end
 
-      it "should ignore the .use_database value when overrride" do
-        @class.use_database('testing')
-        db = @class.prepare_database('test', true)
-        expect(db.name).to eql('couchrest_test')
-      end
     end
 
     describe "protected methods" do

spec/unit/server_pool_spec.rb

@@ -0,0 +1,31 @@
+
+require "spec_helper"
+
+describe CouchRest::Model::ServerPool do
+
+  subject { CouchRest::Model::ServerPool }
+
+  describe ".instance" do
+
+    it "should provide a singleton" do 
+      expect(subject.instance).to be_a(CouchRest::Model::ServerPool)
+    end
+
+  end
+
+  describe "#[url]" do
+
+    it "should provide a server object" do
+      srv = subject.instance[COUCHHOST]
+      expect(srv).to be_a(CouchRest::Server)
+    end
+
+    it "should always provide same object" do
+      srv = subject.instance[COUCHHOST]
+      srv2 = subject.instance[COUCHHOST]
+      expect(srv.object_id).to eql(srv2.object_id)
+    end
+
+  end
+
+end