diff --git a/spec/custom_drivers_types_spec.cr b/spec/custom_drivers_types_spec.cr index f0fc8f1..8f4a0ca 100644 --- a/spec/custom_drivers_types_spec.cr +++ b/spec/custom_drivers_types_spec.cr @@ -48,9 +48,13 @@ class FooDriver < DB::Driver end class FooConnection < DB::Connection - def build_statement(query) + def build_prepared_statement(query) FooStatement.new(self) end + + def build_unprepared_statement(query) + raise "not implemented" + end end class FooStatement < DB::Statement @@ -107,9 +111,13 @@ class BarDriver < DB::Driver end class BarConnection < DB::Connection - def build_statement(query) + def build_prepared_statement(query) BarStatement.new(self) end + + def build_unprepared_statement(query) + raise "not implemented" + end end class BarStatement < DB::Statement diff --git a/spec/database_spec.cr b/spec/database_spec.cr index 9a42425..3915a02 100644 --- a/spec/database_spec.cr +++ b/spec/database_spec.cr @@ -42,24 +42,24 @@ describe DB::Database do it "should allow creation of more statements than pool connections" do DB.open "dummy://localhost:1027?initial_pool_size=1&max_pool_size=2" do |db| - db.prepare("query1").should be_a(DB::PoolStatement) - db.prepare("query2").should be_a(DB::PoolStatement) - db.prepare("query3").should be_a(DB::PoolStatement) + db.build("query1").should be_a(DB::PoolPreparedStatement) + db.build("query2").should be_a(DB::PoolPreparedStatement) + db.build("query3").should be_a(DB::PoolPreparedStatement) end end it "should return same statement in pool per query" do with_dummy do |db| - stmt = db.prepare("query1") - db.prepare("query2").should_not eq(stmt) - db.prepare("query1").should eq(stmt) + stmt = db.build("query1") + db.build("query2").should_not eq(stmt) + db.build("query1").should eq(stmt) end end it "should close pool statements when closing db" do stmt = uninitialized DB::PoolStatement with_dummy do |db| - stmt = db.prepare("query1") + stmt = db.build("query1") end stmt.closed?.should be_true end @@ -97,4 +97,86 @@ describe DB::Database do DummyDriver::DummyConnection.connections.size.should eq(2) end end + + describe "prepared_statements connection option" do + it "defaults to true" do + with_dummy "dummy://localhost:1027" do |db| + db.prepared_statements?.should be_true + end + end + + it "can be set to false" do + with_dummy "dummy://localhost:1027?prepared_statements=false" do |db| + db.prepared_statements?.should be_false + end + end + + it "is copied to connections (false)" do + with_dummy "dummy://localhost:1027?prepared_statements=false&initial_pool_size=1" do |db| + connection = DummyDriver::DummyConnection.connections.first + connection.prepared_statements?.should be_false + end + end + + it "is copied to connections (true)" do + with_dummy "dummy://localhost:1027?prepared_statements=true&initial_pool_size=1" do |db| + connection = DummyDriver::DummyConnection.connections.first + connection.prepared_statements?.should be_true + end + end + + it "should build prepared statements if true" do + with_dummy "dummy://localhost:1027?prepared_statements=true" do |db| + db.build("the query").should be_a(DB::PoolPreparedStatement) + end + end + + it "should build unprepared statements if false" do + with_dummy "dummy://localhost:1027?prepared_statements=false" do |db| + db.build("the query").should be_a(DB::PoolUnpreparedStatement) + end + end + + it "should be overrided by dsl" do + with_dummy "dummy://localhost:1027?prepared_statements=true" do |db| + stmt = db.unprepared.query("the query").statement.as(DummyDriver::DummyStatement) + stmt.prepared?.should be_false + end + + with_dummy "dummy://localhost:1027?prepared_statements=false" do |db| + stmt = db.prepared.query("the query").statement.as(DummyDriver::DummyStatement) + stmt.prepared?.should be_true + end + end + end + + describe "unprepared statements in pool" do + it "creating statements should not create new connections" do + with_dummy "dummy://localhost:1027?initial_pool_size=1" do |db| + stmt1 = db.unprepared.build("query1") + stmt2 = db.unprepared.build("query2") + DummyDriver::DummyConnection.connections.size.should eq(1) + end + end + + it "simultaneous statements should go to different connections" do + with_dummy "dummy://localhost:1027?initial_pool_size=1" do |db| + rs1 = db.unprepared.query("query1") + rs2 = db.unprepared.query("query2") + rs1.statement.connection.should_not eq(rs2.statement.connection) + DummyDriver::DummyConnection.connections.size.should eq(2) + end + end + + it "sequential statements should go to different connections" do + with_dummy "dummy://localhost:1027?initial_pool_size=1" do |db| + rs1 = db.unprepared.query("query1") + rs1.close + rs2 = db.unprepared.query("query2") + rs2.close + rs1.statement.connection.should eq(rs2.statement.connection) + DummyDriver::DummyConnection.connections.size.should eq(1) + end + end + end end diff --git a/spec/db_spec.cr b/spec/db_spec.cr index 0ba8f29..c619c9d 100644 --- a/spec/db_spec.cr +++ b/spec/db_spec.cr @@ -97,4 +97,19 @@ describe DB do DB.open "foobar://baz" end end + + it "should parse boolean query string params" do + DB.fetch_bool(HTTP::Params.parse("foo=true"), "foo", false).should be_true + DB.fetch_bool(HTTP::Params.parse("foo=True"), "foo", false).should be_true + + DB.fetch_bool(HTTP::Params.parse("foo=false"), "foo", true).should be_false + DB.fetch_bool(HTTP::Params.parse("foo=False"), "foo", true).should be_false + + DB.fetch_bool(HTTP::Params.parse("bar=true"), "foo", false).should be_false + DB.fetch_bool(HTTP::Params.parse("bar=true"), "foo", true).should be_true + + expect_raises(ArgumentError, %(invalid "other" value for option "foo")) do + DB.fetch_bool(HTTP::Params.parse("foo=other"), "foo", true) + end + end end diff --git a/spec/dummy_driver.cr b/spec/dummy_driver.cr index fbd02e0..426fe74 100644 --- a/spec/dummy_driver.cr +++ b/spec/dummy_driver.cr @@ -22,8 +22,12 @@ class DummyDriver < DB::Driver @@connections.try &.clear end - def build_statement(query) - DummyStatement.new(self, query) + def build_prepared_statement(query) + DummyStatement.new(self, query, true) + end + + def build_unprepared_statement(query) + DummyStatement.new(self, query, false) end def last_insert_id : Int64 @@ -46,7 +50,7 @@ class DummyDriver < DB::Driver class DummyStatement < DB::Statement property params - def initialize(connection, @query : String) + def initialize(connection, @query : String, @prepared : Bool) @params = Hash(Int32 | String, DB::Any).new super(connection) end @@ -79,6 +83,10 @@ class DummyDriver < DB::Driver raise "not implemented for #{value.class}" end + def prepared? + @prepared + end + protected def do_close super end @@ -204,8 +212,8 @@ def with_dummy(uri : String = "dummy://host?checkout_timeout=0.5") end end -def with_dummy_connection - with_dummy do |db| +def with_dummy_connection(options = "") + with_dummy("dummy://host?checkout_timeout=0.5&#{options}") do |db| db.using_connection do |cnn| yield cnn.as(DummyDriver::DummyConnection) end diff --git a/spec/statement_spec.cr b/spec/statement_spec.cr index ad7b4b7..70c3385 100644 --- a/spec/statement_spec.cr +++ b/spec/statement_spec.cr @@ -1,15 +1,41 @@ require "./spec_helper" describe DB::Statement do - it "should prepare statements" do + it "should build prepared statements" do with_dummy_connection do |cnn| - cnn.prepare("the query").should be_a(DB::Statement) + prepared = cnn.prepared("the query") + prepared.should be_a(DB::Statement) + prepared.as(DummyDriver::DummyStatement).prepared?.should be_true + end + end + + it "should build unprepared statements" do + with_dummy_connection("prepared_statements=false") do |cnn| + prepared = cnn.unprepared("the query") + prepared.should be_a(DB::Statement) + prepared.as(DummyDriver::DummyStatement).prepared?.should be_false + end + end + + describe "prepared_statements flag" do + it "should build prepared statements if true" do + with_dummy_connection("prepared_statements=true") do |cnn| + stmt = cnn.query("the query").statement + stmt.as(DummyDriver::DummyStatement).prepared?.should be_true + end + end + + it "should build unprepared statements if false" do + with_dummy_connection("prepared_statements=false") do |cnn| + stmt = cnn.query("the query").statement + stmt.as(DummyDriver::DummyStatement).prepared?.should be_false + end end end it "should initialize positional params in query" do with_dummy_connection do |cnn| - stmt = cnn.prepare("the query").as(DummyDriver::DummyStatement) + stmt = cnn.prepared("the query").as(DummyDriver::DummyStatement) stmt.query "a", 1, nil stmt.params[0].should eq("a") stmt.params[1].should eq(1) @@ -19,7 +45,7 @@ describe DB::Statement do it "should initialize positional params in query with array" do with_dummy_connection do |cnn| - stmt = cnn.prepare("the query").as(DummyDriver::DummyStatement) + stmt = cnn.prepared("the query").as(DummyDriver::DummyStatement) stmt.query ["a", 1, nil] stmt.params[0].should eq("a") stmt.params[1].should eq(1) @@ -29,7 +55,7 @@ describe DB::Statement do it "should initialize positional params in exec" do with_dummy_connection do |cnn| - stmt = cnn.prepare("the query").as(DummyDriver::DummyStatement) + stmt = cnn.prepared("the query").as(DummyDriver::DummyStatement) stmt.exec "a", 1, nil stmt.params[0].should eq("a") stmt.params[1].should eq(1) @@ -39,7 +65,7 @@ describe DB::Statement do it "should initialize positional params in exec with array" do with_dummy_connection do |cnn| - stmt = cnn.prepare("the query").as(DummyDriver::DummyStatement) + stmt = cnn.prepared("the query").as(DummyDriver::DummyStatement) stmt.exec ["a", 1, nil] stmt.params[0].should eq("a") stmt.params[1].should eq(1) @@ -49,7 +75,7 @@ describe DB::Statement do it "should initialize positional params in scalar" do with_dummy_connection do |cnn| - stmt = cnn.prepare("the query").as(DummyDriver::DummyStatement) + stmt = cnn.prepared("the query").as(DummyDriver::DummyStatement) stmt.scalar "a", 1, nil stmt.params[0].should eq("a") stmt.params[1].should eq(1) @@ -59,7 +85,7 @@ describe DB::Statement do it "query with block should not close statement" do with_dummy_connection do |cnn| - stmt = cnn.prepare "3,4 1,2" + stmt = cnn.prepared "3,4 1,2" stmt.query stmt.closed?.should be_false end @@ -68,7 +94,7 @@ describe DB::Statement do it "closing connection should close statement" do stmt = uninitialized DB::Statement with_dummy_connection do |cnn| - stmt = cnn.prepare "3,4 1,2" + stmt = cnn.prepared "3,4 1,2" stmt.query end stmt.closed?.should be_true @@ -76,7 +102,7 @@ describe DB::Statement do it "query with block should not close statement" do with_dummy_connection do |cnn| - stmt = cnn.prepare "3,4 1,2" + stmt = cnn.prepared "3,4 1,2" stmt.query do |rs| end stmt.closed?.should be_false @@ -85,7 +111,7 @@ describe DB::Statement do it "query should not close statement" do with_dummy_connection do |cnn| - stmt = cnn.prepare "3,4 1,2" + stmt = cnn.prepared "3,4 1,2" stmt.query do |rs| end stmt.closed?.should be_false @@ -94,7 +120,7 @@ describe DB::Statement do it "scalar should not close statement" do with_dummy_connection do |cnn| - stmt = cnn.prepare "3,4 1,2" + stmt = cnn.prepared "3,4 1,2" stmt.scalar stmt.closed?.should be_false end @@ -102,7 +128,7 @@ describe DB::Statement do it "exec should not close statement" do with_dummy_connection do |cnn| - stmt = cnn.prepare "3,4 1,2" + stmt = cnn.prepared "3,4 1,2" stmt.exec stmt.closed?.should be_false end @@ -110,11 +136,11 @@ describe DB::Statement do it "connection should cache statements by query" do with_dummy_connection do |cnn| - rs = cnn.query "1, ?", 2 + rs = cnn.prepared.query "1, ?", 2 stmt = rs.statement rs.close - rs = cnn.query "1, ?", 4 + rs = cnn.prepared.query "1, ?", 4 rs.statement.should be(stmt) end end diff --git a/src/db.cr b/src/db.cr index 9e2979c..74e3d98 100644 --- a/src/db.cr +++ b/src/db.cr @@ -120,17 +120,34 @@ module DB private def self.build_database(uri : URI) Database.new(driver_class(uri.scheme).new, uri) end + + # :nodoc: + def self.fetch_bool(params : HTTP::Params, name, default : Bool) + case (value = params[name]?).try &.downcase + when nil + default + when "true" + true + when "false" + false + else + raise ArgumentError.new(%(invalid "#{value}" value for option "#{name}")) + end + end end require "./db/pool" require "./db/string_key_cache" require "./db/query_methods" +require "./db/session_methods" require "./db/disposable" -require "./db/database" require "./db/driver" -require "./db/connection" require "./db/statement" +require "./db/connection" require "./db/pool_statement" +require "./db/database" +require "./db/pool_prepared_statement" +require "./db/pool_unprepared_statement" require "./db/result_set" require "./db/error" require "./db/mapping" diff --git a/src/db/connection.cr b/src/db/connection.cr index d6a8ee2..803ebd8 100644 --- a/src/db/connection.cr +++ b/src/db/connection.cr @@ -11,7 +11,8 @@ module DB # # The connection must be initialized in `#initialize` and closed in `#do_close`. # - # Override `#build_statement` method in order to return a prepared `Statement` to allow querying. + # Override `#build_prepared_statement` method in order to return a prepared `Statement` to allow querying. + # Override `#build_unprepared_statement` method in order to return a unprepared `Statement` to allow querying. # See also `Statement` to define how the statements are executed. # # If at any give moment the connection is lost a DB::ConnectionLost should be raised. This will @@ -19,21 +20,27 @@ module DB # abstract class Connection include Disposable - include QueryMethods + include SessionMethods(Connection, Statement) # :nodoc: getter database @statements_cache = StringKeyCache(Statement).new + getter? prepared_statements : Bool def initialize(@database : Database) + @prepared_statements = @database.prepared_statements? end # :nodoc: - def prepare(query) : Statement - @statements_cache.fetch(query) { build_statement(query) } + def fetch_or_build_prepared_statement(query) + @statements_cache.fetch(query) { build_prepared_statement(query) } end - abstract def build_statement(query) : Statement + # :nodoc: + abstract def build_prepared_statement(query) : Statement + + # :nodoc: + abstract def build_unprepared_statement(query) : Statement protected def do_close @statements_cache.each_value &.close diff --git a/src/db/database.cr b/src/db/database.cr index 668f176..f80e6b8 100644 --- a/src/db/database.cr +++ b/src/db/database.cr @@ -13,10 +13,17 @@ module DB # - retry_attempts (default 1) # - retry_delay (in seconds, default 1.0) # + # When querying a database prepared statements are used by default. + # This can be changed from the `prepared_statements` URI parameter: + # + # - prepared_statements = `true`|`false` (default `true`) + # # It should be created from DB module. See `DB#open`. # - # Refer to `QueryMethods` for documentation about querying the database. + # Refer to `QueryMethods` and `SessionMethods` for documentation about querying the database. class Database + include SessionMethods(Database, PoolStatement) + # :nodoc: getter driver # :nodoc: @@ -25,13 +32,16 @@ module DB # Returns the uri with the connection settings to the database getter uri + getter? prepared_statements : Bool + @pool : Pool(Connection) @setup_connection : Connection -> Nil - @statements_cache = StringKeyCache(PoolStatement).new + @statements_cache = StringKeyCache(PoolPreparedStatement).new # :nodoc: def initialize(@driver : Driver, @uri : URI) params = HTTP::Params.parse(uri.query || "") + @prepared_statements = DB.fetch_bool(params, "prepared_statements", true) pool_options = @driver.connection_pool_options(params) @setup_connection = ->(conn : Connection) {} @@ -59,8 +69,18 @@ module DB end # :nodoc: - def prepare(query) - @statements_cache.fetch(query) { PoolStatement.new(self, query) } + def fetch_or_build_prepared_statement(query) + @statements_cache.fetch(query) { build_prepared_statement(query) } + end + + # :nodoc: + def build_prepared_statement(query) + PoolPreparedStatement.new(self, query) + end + + # :nodoc: + def build_unprepared_statement(query) + PoolUnpreparedStatement.new(self, query) end # :nodoc: @@ -91,7 +111,5 @@ module DB yield end end - - include QueryMethods end end diff --git a/src/db/pool_prepared_statement.cr b/src/db/pool_prepared_statement.cr new file mode 100644 index 0000000..63ed49b --- /dev/null +++ b/src/db/pool_prepared_statement.cr @@ -0,0 +1,50 @@ +module DB + # Represents a statement to be executed in any of the connections + # of the pool. The statement is not be executed in a prepared fashion. + # The execution of the statement is retried according to the pool configuration. + # + # See `PoolStatement` + class PoolPreparedStatement < PoolStatement + # connections where the statement was prepared + @connections = Set(WeakRef(Connection)).new + + def initialize(db : Database, query : String) + super + # Prepares a statement on some connection + # otherwise the preparation is delayed until the first execution. + # After the first initialization the connection must be released + # it will be checked out when executing it. + statement_with_retry &.release_connection + # TODO use a round-robin selection in the pool so multiple sequentially + # initialized statements are assigned to different connections. + end + + protected def do_close + # TODO close all statements on all connections. + # currently statements are closed when the connection is closed. + + # WHAT-IF the connection is busy? Should each statement be able to + # deallocate itself when the connection is free. + @connections.clear + end + + # builds a statement over a real connection + # the conneciton is registered in `@connections` + private def build_statement + clean_connections + conn, existing = @db.checkout_some(@connections) + @connections << WeakRef.new(conn) unless existing + conn.prepared.build(@query) + end + + private def clean_connections + # remove disposed or closed connections + @connections.each do |ref| + conn = ref.target + if !conn || conn.closed? + @connections.delete ref + end + end + end + end +end diff --git a/src/db/pool_statement.cr b/src/db/pool_statement.cr index 6e2354e..5a7b51f 100644 --- a/src/db/pool_statement.cr +++ b/src/db/pool_statement.cr @@ -3,29 +3,10 @@ module DB # a statement from the DB needs to be able to represent a statement in any # of the connections of the pool. Otherwise the user will need to deal with # actual connections in some point. - class PoolStatement + abstract class PoolStatement include StatementMethods - # connections where the statement was prepared - @connections = Set(WeakRef(Connection)).new - def initialize(@db : Database, @query : String) - # Prepares a statement on some connection - # otherwise the preparation is delayed until the first execution. - # After the first initialization the connection must be released - # it will be checked out when executing it. - statement_with_retry &.release_connection - # TODO use a round-robin selection in the pool so multiple sequentially - # initialized statements are assigned to different connections. - end - - protected def do_close - # TODO close all statements on all connections. - # currently statements are closed when the connection is closed. - - # WHAT-IF the connection is busy? Should each statement be able to - # deallocate itself when the connection is free. - @connections.clear end # See `QueryMethods#exec` @@ -60,22 +41,7 @@ module DB # builds a statement over a real connection # the conneciton is registered in `@connections` - private def build_statement - clean_connections - conn, existing = @db.checkout_some(@connections) - @connections << WeakRef.new(conn) unless existing - conn.prepare(@query) - end - - private def clean_connections - # remove disposed or closed connections - @connections.each do |ref| - conn = ref.target - if !conn || conn.closed? - @connections.delete ref - end - end - end + private abstract def build_statement : Statement private def statement_with_retry @db.retry do diff --git a/src/db/pool_unprepared_statement.cr b/src/db/pool_unprepared_statement.cr new file mode 100644 index 0000000..1a120e5 --- /dev/null +++ b/src/db/pool_unprepared_statement.cr @@ -0,0 +1,21 @@ +module DB + # Represents a statement to be executed in any of the connections + # of the pool. The statement is not be executed in a non prepared fashion. + # The execution of the statement is retried according to the pool configuration. + # + # See `PoolStatement` + class PoolUnpreparedStatement < PoolStatement + def initialize(db : Database, query : String) + super + end + + protected def do_close + # unprepared statements do not need to be release in each connection + end + + # builds a statement over a real connection + private def build_statement + @db.pool.checkout.unprepared.build(@query) + end + end +end diff --git a/src/db/query_methods.cr b/src/db/query_methods.cr index f41596b..802a6ad 100644 --- a/src/db/query_methods.cr +++ b/src/db/query_methods.cr @@ -15,11 +15,11 @@ module DB # # Convention of mapping how arguments are mapped to the query depends on each driver. # - # Including `QueryMethods` requires a `prepare(query) : Statement` method that is not expected + # Including `QueryMethods` requires a `build(query) : Statement` method that is not expected # to be called directly. module QueryMethods # :nodoc: - abstract def prepare(query) : Statement + abstract def build(query) : Statement # Executes a *query* and returns a `ResultSet` with the results. # The `ResultSet` must be closed manually. @@ -35,7 +35,7 @@ module DB # end # ``` def query(query, *args) - prepare(query).query(*args) + build(query).query(*args) end # Executes a *query* and yields a `ResultSet` with the results. @@ -49,7 +49,7 @@ module DB # end # ``` def query(query, *args) - # CHECK prepare(query).query(*args, &block) + # CHECK build(query).query(*args, &block) rs = query(query, *args) yield rs ensure rs.close end @@ -200,13 +200,13 @@ module DB # Performs the `query` and returns an `ExecResult` def exec(query, *args) - prepare(query).exec(*args) + build(query).exec(*args) end # Performs the `query` and returns a single scalar value # puts db.scalar("SELECT MAX(name)").as(String) # => (a String) def scalar(query, *args) - prepare(query).scalar(*args) + build(query).scalar(*args) end end end diff --git a/src/db/session_methods.cr b/src/db/session_methods.cr new file mode 100644 index 0000000..1bc189c --- /dev/null +++ b/src/db/session_methods.cr @@ -0,0 +1,73 @@ +module DB + # Methods that are shared accross session like objects: + # - Database + # - Connection + # + # Classes that includes this module are able to execute + # queries and statements in both prepared and unprepared fashion. + # + # This module serves for dsl reuse over session like objects. + module SessionMethods(Session, Stmt) + include QueryMethods + + # Returns whether by default the statements should + # be prepared or not. + abstract def prepared_statements? : Bool + + abstract def fetch_or_build_prepared_statement(query) : Stmt + + abstract def build_unprepared_statement(query) : Stmt + + def build(query) : Stmt + if prepared_statements? + fetch_or_build_prepared_statement(query) + else + build_unprepared_statement(query) + end + end + + # dsl helper to build prepared statements + # returns a value that includes `QueryMethods` + def prepared + PreparedQuery(Session, Stmt).new(self) + end + + # Returns a prepared `Statement` that has not been executed yet. + def prepared(query) + prepared.build(query) + end + + # dsl helper to build unprepared statements + # returns a value that includes `QueryMethods` + def unprepared + UnpreparedQuery(Session, Stmt).new(self) + end + + # Returns an unprepared `Statement` that has not been executed yet. + def unprepared(query) + unprepared.build(query) + end + + struct PreparedQuery(Session, Stmt) + include QueryMethods + + def initialize(@session : Session) + end + + def build(query) : Stmt + @session.fetch_or_build_prepared_statement(query) + end + end + + struct UnpreparedQuery(Session, Stmt) + include QueryMethods + + def initialize(@session : Session) + end + + def build(query) : Stmt + @session.build_unprepared_statement(query) + end + end + end +end diff --git a/src/db/statement.cr b/src/db/statement.cr index 3f8058b..01eba35 100644 --- a/src/db/statement.cr +++ b/src/db/statement.cr @@ -39,7 +39,7 @@ module DB abstract def query(args : Array) : ResultSet end - # Represents a prepared query in a `Connection`. + # Represents a query in a `Connection`. # It should be created by `QueryMethods`. # # ### Note to implementors