From e54acf1308e2e4df047bf90798208e03e1370098 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
<rafaelmfranca@gmail.com>
Date: Thu, 21 Feb 2013 11:58:33 -0300
Subject: Do not type cast all the database url values.
We should only type cast when we need to use.
Related to 4b005fb371c2e7af80df7da63be94509b1db038c
---
activerecord/CHANGELOG.md | 6 +++---
.../connection_adapters/abstract_adapter.rb | 18 ++++++++++++++++++
.../connection_adapters/abstract_mysql_adapter.rb | 8 ++++----
.../connection_adapters/connection_specification.rb | 21 ---------------------
.../connection_adapters/mysql_adapter.rb | 2 +-
.../connection_adapters/postgresql_adapter.rb | 6 +++---
.../connection_adapters/sqlite3_adapter.rb | 6 +++---
.../cases/connection_specification/resolver_test.rb | 21 ---------------------
8 files changed, 32 insertions(+), 56 deletions(-)
(limited to 'activerecord')
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8e51fdf199..39bbcb795f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -151,8 +151,8 @@
*Justin George*
-* The `DATABASE_URL` environment variable now converts ints, floats, and
- the strings true and false to Ruby types. For example, SQLite requires
+* The database adpters now converts the options passed thought `DATABASE_URL`
+ environment variable to the proper Ruby types before using. For example, SQLite requires
that the timeout value is an integer, and PostgreSQL requires that the
prepared_statements option is a boolean. These now work as expected:
@@ -161,7 +161,7 @@
DATABASE_URL=sqlite3://localhost/test_db?timeout=500
DATABASE_URL=postgresql://localhost/test_db?prepared_statements=false
- *Aaron Stone*
+ *Aaron Stone + Rafael Mendonça França*
* `Relation#merge` now only overwrites where values on the LHS of the
merge. Consider:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 67d4d505f7..ff9de712bc 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -61,12 +61,30 @@ module ActiveRecord
include MonitorMixin
include ColumnDumper
+ SIMPLE_INT = /\A\d+\z/
+
define_callbacks :checkout, :checkin
attr_accessor :visitor, :pool
attr_reader :schema_cache, :last_use, :in_use, :logger
alias :in_use? :in_use
+ def self.type_cast_config_to_integer(config)
+ if config =~ SIMPLE_INT
+ config.to_i
+ else
+ config
+ end
+ end
+
+ def self.type_cast_config_to_boolean(config)
+ if config == "false"
+ false
+ else
+ config
+ end
+ end
+
def initialize(connection, logger = nil, pool = nil) #:nodoc:
super()
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 0871454cfe..5480204511 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -31,7 +31,7 @@ module ActiveRecord
return false if blob_or_text_column? #mysql forbids defaults on blob and text columns
super
end
-
+
def blob_or_text_column?
sql_type =~ /blob/i || type == :text
end
@@ -140,7 +140,7 @@ module ActiveRecord
@connection_options, @config = connection_options, config
@quoted_column_names, @quoted_table_names = {}, {}
- if config.fetch(:prepared_statements) { true }
+ if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
@visitor = Arel::Visitors::MySQL.new self
else
@visitor = BindSubstitution.new self
@@ -581,7 +581,7 @@ module ActiveRecord
end
def strict_mode?
- @config.fetch(:strict, true)
+ self.class.type_cast_config_to_boolean(@config.fetch(:strict, true))
end
protected
@@ -722,7 +722,7 @@ module ActiveRecord
# Increase timeout so the server doesn't disconnect us.
wait_timeout = @config[:wait_timeout]
wait_timeout = 2147483 unless wait_timeout.is_a?(Fixnum)
- variables[:wait_timeout] = wait_timeout
+ variables[:wait_timeout] = self.class.type_cast_config_to_integer(wait_timeout)
# Make MySQL reject illegal values rather than truncating or blanking them, see
# http://dev.mysql.com/doc/refman/5.0/en/server-sql-mode.html#sqlmode_strict_all_tables
diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
index de418eab14..2c683fc3ac 100644
--- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb
+++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb
@@ -65,10 +65,6 @@ module ActiveRecord
ConnectionSpecification.new(spec, adapter_method)
end
- # For DATABASE_URL, accept a limited concept of ints and floats
- SIMPLE_INT = /\A\d+\z/
- SIMPLE_FLOAT = /\A\d+\.\d+\z/
-
def self.connection_url_to_hash(url) # :nodoc:
config = URI.parse url
adapter = config.scheme
@@ -89,28 +85,11 @@ module ActiveRecord
if config.query
options = Hash[config.query.split("&").map{ |pair| pair.split("=") }].symbolize_keys
- options.each { |key, value| options[key] = type_cast_value(value) }
-
spec.merge!(options)
end
spec
end
-
- def self.type_cast_value(value)
- case value
- when SIMPLE_INT
- value.to_i
- when SIMPLE_FLOAT
- value.to_f
- when 'true'
- true
- when 'false'
- false
- else
- value
- end
- end
end
end
end
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 631f646f58..7544c2a783 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -126,7 +126,7 @@ module ActiveRecord
def initialize(connection, logger, connection_options, config)
super
@statements = StatementPool.new(@connection,
- config.fetch(:statement_limit) { 1000 })
+ self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 }))
@client_encoding = nil
connect
end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 0818760b11..2bb2557efd 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -478,7 +478,7 @@ module ActiveRecord
def initialize(connection, logger, connection_parameters, config)
super(connection, logger)
- if config.fetch(:prepared_statements) { true }
+ if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
@visitor = Arel::Visitors::PostgreSQL.new self
else
@visitor = BindSubstitution.new self
@@ -492,7 +492,7 @@ module ActiveRecord
connect
@statements = StatementPool.new @connection,
- config.fetch(:statement_limit) { 1000 }
+ self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 })
if postgresql_version < 80200
raise "Your version of PostgreSQL (#{postgresql_version}) is too old, please upgrade!"
@@ -500,7 +500,7 @@ module ActiveRecord
initialize_type_map
@local_tz = execute('SHOW TIME ZONE', 'SCHEMA').first["TimeZone"]
- @use_insert_returning = @config.key?(:insert_returning) ? @config[:insert_returning] : true
+ @use_insert_returning = @config.key?(:insert_returning) ? self.class.type_cast_config_to_boolean(@config[:insert_returning]) : true
end
# Clears the prepared statements cache.
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 6ede5f3390..981c4c96a0 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -26,7 +26,7 @@ module ActiveRecord
:results_as_hash => true
)
- db.busy_timeout(config[:timeout]) if config[:timeout]
+ db.busy_timeout(ConnectionAdapters::SQLite3Adapter.type_cast_config_to_integer(config[:timeout])) if config[:timeout]
ConnectionAdapters::SQLite3Adapter.new(db, logger, config)
end
@@ -107,10 +107,10 @@ module ActiveRecord
@active = nil
@statements = StatementPool.new(@connection,
- config.fetch(:statement_limit) { 1000 })
+ self.class.type_cast_config_to_integer(config.fetch(:statement_limit) { 1000 }))
@config = config
- if config.fetch(:prepared_statements) { true }
+ if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
@visitor = Arel::Visitors::SQLite.new self
else
@visitor = BindSubstitution.new self
diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/connection_specification/resolver_test.rb
index 0a04117795..c8dfc3244b 100644
--- a/activerecord/test/cases/connection_specification/resolver_test.rb
+++ b/activerecord/test/cases/connection_specification/resolver_test.rb
@@ -43,27 +43,6 @@ module ActiveRecord
encoding: "utf8" }, spec)
end
- def test_url_query_numeric
- spec = resolve 'abstract://foo:123?encoding=utf8&int=500&float=10.9'
- assert_equal({
- adapter: "abstract",
- port: 123,
- int: 500,
- float: 10.9,
- host: "foo",
- encoding: "utf8" }, spec)
- end
-
- def test_url_query_boolean
- spec = resolve 'abstract://foo:123?true=true&false=false'
- assert_equal({
- adapter: "abstract",
- port: 123,
- true: true,
- false: false,
- host: "foo" }, spec)
- end
-
def test_encoded_password
password = 'am@z1ng_p@ssw0rd#!'
encoded_password = URI.encode_www_form_component(password)
--
cgit v1.2.3