From: Michael Orlitzky Date: Sun, 8 Nov 2015 04:37:51 +0000 (-0500) Subject: Wrap all close() calls in "ensure" blocks and simplify DB connection-making. X-Git-Tag: 0.0.1~9 X-Git-Url: https://gitweb.michael.orlitzky.com/?a=commitdiff_plain;h=b947ef8844f090eedd50be0383abe417d910bb1a;p=mailshears.git Wrap all close() calls in "ensure" blocks and simplify DB connection-making. --- diff --git a/doc/TODO b/doc/TODO index 37c06ce..bd750c9 100644 --- a/doc/TODO +++ b/doc/TODO @@ -17,8 +17,6 @@ * Make a release. -* Should we wrap all of the connection.close() calls in "ensure"? - * Test deletion of a user "bar" who lies in the middle of a goto="foo,bar,baz" alias. diff --git a/lib/common/agendav_plugin.rb b/lib/common/agendav_plugin.rb index 8eb73c1..49e7087 100644 --- a/lib/common/agendav_plugin.rb +++ b/lib/common/agendav_plugin.rb @@ -14,13 +14,14 @@ module AgendavPlugin # @param cfg [Configuration] the configuration for this plugin. # def initialize(cfg) - @db_host = cfg.agendav_dbhost - @db_port = cfg.agendav_dbport - @db_opts = cfg.agendav_dbopts - @db_tty = cfg.agendav_dbtty - @db_name = cfg.agendav_dbname - @db_user = cfg.agendav_dbuser - @db_pass = cfg.agendav_dbpass + @db_hash = { + :host => cfg.agendav_dbhost, + :port => cfg.agendav_dbport, + :options => cfg.agendav_dbopts, + :tty => cfg.agendav_dbtty, + :dbname => cfg.agendav_dbname, + :user => cfg.agendav_dbuser, + :password => cfg.agendav_dbpass } end @@ -32,19 +33,21 @@ module AgendavPlugin def list_users() users = [] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) sql_query = '(SELECT username FROM prefs)' sql_query += 'UNION' sql_query += '(SELECT user_from FROM shared);' - connection.query(sql_query) do |result| - users = result.field_values('username') + begin + connection.query(sql_query) do |result| + users = result.field_values('username') + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - connection.close() - return users.map{ |u| User.new(u) } end diff --git a/lib/common/davical_plugin.rb b/lib/common/davical_plugin.rb index 8684cca..ba09770 100644 --- a/lib/common/davical_plugin.rb +++ b/lib/common/davical_plugin.rb @@ -14,13 +14,14 @@ module DavicalPlugin # @param cfg [Configuration] the configuration for this plugin. # def initialize(cfg) - @db_host = cfg.davical_dbhost - @db_port = cfg.davical_dbport - @db_opts = cfg.davical_dbopts - @db_tty = cfg.davical_dbtty - @db_name = cfg.davical_dbname - @db_user = cfg.davical_dbuser - @db_pass = cfg.davical_dbpass + @db_hash = { + :host => cfg.davical_dbhost, + :port => cfg.davical_dbport, + :options => cfg.davical_dbopts, + :tty => cfg.davical_dbtty, + :dbname => cfg.davical_dbname, + :user => cfg.davical_dbuser, + :password => cfg.davical_dbpass } end @@ -48,18 +49,20 @@ module DavicalPlugin def list_users() usernames = [] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) # User #1 is the super-user, and not tied to an email address. - sql_query = 'SELECT username FROM usr WHERE user_no > 1' + sql_query = 'SELECT username FROM usr WHERE user_no > 1;' - connection.query(sql_query) do |result| - usernames = result.field_values('username') + begin + connection.query(sql_query) do |result| + usernames = result.field_values('username') + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - connection.close() - return usernames.map{ |u| User.new(u) } end @@ -77,22 +80,24 @@ module DavicalPlugin def get_principal_id(user) principal_id = nil - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) sql_query = 'SELECT principal.principal_id ' sql_query += 'FROM (principal INNER JOIN usr ' sql_query += ' ON principal.user_no = usr.user_no) ' sql_query += 'WHERE usr.username = $1;' - connection.query(sql_query, [user.to_s()]) do |result| - if result.num_tuples > 0 - principal_id = result[0]['principal_id'] + begin + connection.query(sql_query, [user.to_s()]) do |result| + if result.num_tuples > 0 + principal_id = result[0]['principal_id'] + end end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - connection.close() - return principal_id end diff --git a/lib/common/postfixadmin_plugin.rb b/lib/common/postfixadmin_plugin.rb index 8bf03f9..08fca7d 100644 --- a/lib/common/postfixadmin_plugin.rb +++ b/lib/common/postfixadmin_plugin.rb @@ -16,13 +16,14 @@ module PostfixadminPlugin # @param cfg [Configuration] the configuration for this plugin. # def initialize(cfg) - @db_host = cfg.postfixadmin_dbhost - @db_port = cfg.postfixadmin_dbport - @db_opts = cfg.postfixadmin_dbopts - @db_tty = cfg.postfixadmin_dbtty - @db_name = cfg.postfixadmin_dbname - @db_user = cfg.postfixadmin_dbuser - @db_pass = cfg.postfixadmin_dbpass + @db_hash = { + :host => cfg.postfixadmin_dbhost, + :port => cfg.postfixadmin_dbport, + :options => cfg.postfixadmin_dbopts, + :tty => cfg.postfixadmin_dbtty, + :dbname => cfg.postfixadmin_dbname, + :user => cfg.postfixadmin_dbuser, + :password => cfg.postfixadmin_dbpass } end @@ -36,16 +37,19 @@ module PostfixadminPlugin def list_domains() domains = [] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) # 'ALL' is a magic domain, and we don't want it. sql_query = "SELECT domain FROM domain WHERE domain <> 'ALL';" - connection.query(sql_query) do |result| - domains = result.field_values('domain') - end - connection.close() + begin + connection.query(sql_query) do |result| + domains = result.field_values('domain') + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() + end return domains.map{ |d| Domain.new(d) } end @@ -59,15 +63,18 @@ module PostfixadminPlugin def list_users() users = [] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) sql_query = 'SELECT username FROM mailbox;' - connection.query(sql_query) do |result| - users = result.field_values('username') - end - connection.close() + begin + connection.query(sql_query) do |result| + users = result.field_values('username') + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() + end return users.map{ |u| User.new(u) } end @@ -86,8 +93,7 @@ module PostfixadminPlugin usernames = [] return usernames if domains.length() == 0 - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) # The number of parameters that we'll pass into our prepared query # is the number of domains that we're given. It's important that @@ -95,14 +101,17 @@ module PostfixadminPlugin params = 1.upto(domains.length()).map{ |i| '$' + i.to_s() }.join(',') sql_query = "SELECT username FROM mailbox WHERE domain IN (#{params});" - # Now replace each Domain with its string representation and pass - # those in as our individual parameters. - connection.query(sql_query, domains.map{ |d| d.to_s() }) do |result| - usernames = result.field_values('username') + begin + # Now replace each Domain with its string representation and pass + # those in as our individual parameters. + connection.query(sql_query, domains.map{ |d| d.to_s() }) do |result| + usernames = result.field_values('username') + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - connection.close() - return usernames.map{ |u| User.new(u) } end @@ -117,17 +126,20 @@ module PostfixadminPlugin def list_aliases() aliases = [] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) sql_query = 'SELECT address,goto FROM alias;' - results = connection.query(sql_query) - results.each do |row| - # row should be a hash - aliases << row - end - connection.close() + begin + results = connection.query(sql_query) + results.each do |row| + # row should be a hash + aliases << row + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() + end return aliases end @@ -146,18 +158,21 @@ module PostfixadminPlugin def domain_exists(domain) count = 0 - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) sql_query = 'SELECT COUNT(domain) as count FROM domain WHERE domain = $1;' - connection.query(sql_query, [domain.to_s()]) do |result| - return false if result.ntuples() < 1 - count = result.getvalue(0,0).to_i() - return false if count.nil? - end + begin + connection.query(sql_query, [domain.to_s()]) do |result| + return false if result.ntuples() < 1 + count = result.getvalue(0,0).to_i() - connection.close() + return false if count.nil? + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() + end return (count > 0) end diff --git a/lib/common/roundcube_plugin.rb b/lib/common/roundcube_plugin.rb index 031e3f9..a4300c4 100644 --- a/lib/common/roundcube_plugin.rb +++ b/lib/common/roundcube_plugin.rb @@ -15,13 +15,14 @@ module RoundcubePlugin # @param cfg [Configuration] the configuration for this plugin. # def initialize(cfg) - @db_host = cfg.roundcube_dbhost - @db_port = cfg.roundcube_dbport - @db_opts = cfg.roundcube_dbopts - @db_tty = cfg.roundcube_dbtty - @db_name = cfg.roundcube_dbname - @db_user = cfg.roundcube_dbuser - @db_pass = cfg.roundcube_dbpass + @db_hash = { + :host => cfg.roundcube_dbhost, + :port => cfg.roundcube_dbport, + :options => cfg.roundcube_dbopts, + :tty => cfg.roundcube_dbtty, + :dbname => cfg.roundcube_dbname, + :user => cfg.roundcube_dbuser, + :password => cfg.roundcube_dbpass } end @@ -46,15 +47,18 @@ module RoundcubePlugin def list_users() usernames = [] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.connect(@db_hash) sql_query = 'SELECT username FROM users;' - connection.query(sql_query) do |result| - usernames = result.field_values('username') - end - connection.close() + begin + connection.query(sql_query) do |result| + usernames = result.field_values('username') + end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() + end return usernames.map{ |u| User.new(u) } end @@ -71,19 +75,20 @@ module RoundcubePlugin def get_user_id(user) user_id = nil - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - + connection = PG::Connection.new(@db_hash) sql_query = 'SELECT user_id FROM users WHERE username = $1;' - connection.query(sql_query, [user.to_s()]) do |result| - if result.num_tuples > 0 - user_id = result[0]['user_id'] + begin + connection.query(sql_query, [user.to_s()]) do |result| + if result.num_tuples > 0 + user_id = result[0]['user_id'] + end end + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - connection.close() - return user_id end diff --git a/lib/mv/plugins/agendav.rb b/lib/mv/plugins/agendav.rb index 80ab1b6..9623c40 100644 --- a/lib/mv/plugins/agendav.rb +++ b/lib/mv/plugins/agendav.rb @@ -32,14 +32,15 @@ class AgendavMv sql_queries << 'UPDATE shared SET user_from = $1 WHERE user_from = $2;' sql_queries << 'UPDATE shared SET user_which = $1 WHERE user_which = $2;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| - connection.query(sql_query, [dst.to_s(), src.to_s()]) + connection = PG::Connection.new(@db_hash) + begin + sql_queries.each do |sql_query| + connection.query(sql_query, [dst.to_s(), src.to_s()]) + end + ensure + # Make sure the connection gets closed even if a query explodes. + connection.close() end - - connection.close() end end diff --git a/lib/mv/plugins/davical.rb b/lib/mv/plugins/davical.rb index 58287b3..e7aa050 100644 --- a/lib/mv/plugins/davical.rb +++ b/lib/mv/plugins/davical.rb @@ -29,16 +29,15 @@ class DavicalMv raise NonexistentUserError.new(src.to_s()) if not user_exists(src) raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) - sql_queries = ['UPDATE usr SET username = $1 WHERE username = $2'] + sql_query = 'UPDATE usr SET username = $1 WHERE username = $2;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| + connection = PG::Connection.new(@db_hash) + begin connection.query(sql_query, [dst.to_s(), src.to_s()]) + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - - connection.close() end end diff --git a/lib/mv/plugins/postfixadmin.rb b/lib/mv/plugins/postfixadmin.rb index e22df6a..f8f86c6 100644 --- a/lib/mv/plugins/postfixadmin.rb +++ b/lib/mv/plugins/postfixadmin.rb @@ -51,20 +51,20 @@ class PostfixadminMv sql_queries = [mailbox_query, alias_query1, alias_query2] - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| - varchar = 1043 # from pg_type.h - params = [{:value => dst.to_s(), :type => varchar}, - {:value => dst.domainpart(), :type => varchar}, - {:value => dst.localpart(), :type => varchar}, - {:value => src.to_s(), :type => varchar}] - - connection.query(sql_query, params) + connection = PG::Connection.new(@db_hash) + begin + sql_queries.each do |sql_query| + varchar = 1043 # from pg_type.h + params = [{:value => dst.to_s(), :type => varchar}, + {:value => dst.domainpart(), :type => varchar}, + {:value => dst.localpart(), :type => varchar}, + {:value => src.to_s(), :type => varchar}] + connection.query(sql_query, params) + end + ensure + # Make sure the connection gets closed even if a query explodes. + connection.close() end - - connection.close() end end diff --git a/lib/mv/plugins/roundcube.rb b/lib/mv/plugins/roundcube.rb index 4d81964..5cd20c7 100644 --- a/lib/mv/plugins/roundcube.rb +++ b/lib/mv/plugins/roundcube.rb @@ -30,16 +30,15 @@ class RoundcubeMv raise NonexistentUserError.new(src.to_s()) if not user_exists(src) raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst) - sql_queries = ['UPDATE users SET username = $1 WHERE username = $2;'] + sql_query = 'UPDATE users SET username = $1 WHERE username = $2;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| + connection = PG::Connection.new(@db_hash) + begin connection.query(sql_query, [dst.to_s(), src.to_s()]) + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - - connection.close() end end diff --git a/lib/rm/plugins/agendav.rb b/lib/rm/plugins/agendav.rb index 28f1eb4..46bd407 100644 --- a/lib/rm/plugins/agendav.rb +++ b/lib/rm/plugins/agendav.rb @@ -24,14 +24,15 @@ class AgendavRm sql_queries = ['DELETE FROM prefs WHERE username = $1;'] sql_queries << 'DELETE FROM shared WHERE user_from = $1;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| - connection.query(sql_query, [user.to_s()]) + connection = PG::Connection.new(@db_hash) + begin + sql_queries.each do |sql_query| + connection.query(sql_query, [user.to_s()]) + end + ensure + # Make sure the connection gets closed even if a query explodes. + connection.close() end - - connection.close() end end diff --git a/lib/rm/plugins/davical.rb b/lib/rm/plugins/davical.rb index 852d9c4..1029994 100644 --- a/lib/rm/plugins/davical.rb +++ b/lib/rm/plugins/davical.rb @@ -23,16 +23,15 @@ class DavicalRm def remove_user(user) raise NonexistentUserError.new(user.to_s()) if not user_exists(user) - sql_queries = ['DELETE FROM usr WHERE username = $1'] + sql_query = 'DELETE FROM usr WHERE username = $1;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| + connection = PG::Connection.new(@db_hash) + begin connection.query(sql_query, [user.to_s()]) + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - - connection.close() end diff --git a/lib/rm/plugins/postfixadmin.rb b/lib/rm/plugins/postfixadmin.rb index b950b7a..c8e44ab 100644 --- a/lib/rm/plugins/postfixadmin.rb +++ b/lib/rm/plugins/postfixadmin.rb @@ -44,21 +44,23 @@ class PostfixadminRm # Should be handled by a trigger, according to PostfixAdmin code. sql_queries << 'DELETE FROM vacation_notification WHERE on_vacation = $1;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) - - sql_queries.each do |sql_query| - connection.query(sql_query, [user.to_s()]) + connection = PG::Connection.new(@db_hash) + + begin + sql_queries.each do |sql_query| + connection.query(sql_query, [user.to_s()]) + end + + # The earlier alias update query will leave things like + # "foo@example.com,,bar@example.com" in the "goto" column. Now + # we fix it. We don't do it in the loop because query() craps + # out on the superfluous parameter. + sql_query = "UPDATE alias SET goto=REPLACE(goto, ',,', ',');" + connection.query(sql_query) + ensure + # Make sure the connection gets closed even if a query explodes. + connection.close() end - - # The earlier alias update query will leave things like - # "foo@example.com,,bar@example.com" in the "goto" column. Now - # we fix it. We don't do it in the loop because query() craps - # out on the superfluous parameter. - sql_query = "UPDATE alias SET goto=REPLACE(goto, ',,', ',');" - connection.query(sql_query) - - connection.close() end @@ -96,14 +98,16 @@ class PostfixadminRm sql_queries << 'DELETE FROM vacation WHERE domain = $1;' sql_queries << 'DELETE FROM domain WHERE domain = $1;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.new(@db_hash) - sql_queries.each do |sql_query| - connection.query(sql_query, [domain.to_s()]) + begin + sql_queries.each do |sql_query| + connection.query(sql_query, [domain.to_s()]) + end + ensure + # Make sure the connection gets closed even if a query explodes. + connection.close() end - - connection.close() end end diff --git a/lib/rm/plugins/roundcube.rb b/lib/rm/plugins/roundcube.rb index 88e88c4..c7ccaa4 100644 --- a/lib/rm/plugins/roundcube.rb +++ b/lib/rm/plugins/roundcube.rb @@ -27,16 +27,16 @@ class RoundcubeRm # Thanks to the ON DELETE triggers, this will remove all child # records associated with user_id too. - sql_queries = ['DELETE FROM users WHERE user_id = $1::int;'] + sql_query = 'DELETE FROM users WHERE user_id = $1::int;' - connection = PGconn.connect(@db_host, @db_port, @db_opts, @db_tty, - @db_name, @db_user, @db_pass) + connection = PG::Connection.connect(@db_hash) - sql_queries.each do |sql_query| + begin connection.query(sql_query, [user_id]) + ensure + # Make sure the connection gets closed even if the query explodes. + connection.close() end - - connection.close() end end diff --git a/test/mailshears_test.rb b/test/mailshears_test.rb index 8e80c06..96a6e35 100644 --- a/test/mailshears_test.rb +++ b/test/mailshears_test.rb @@ -32,8 +32,8 @@ class MailshearsTest < MiniTest::Unit::TestCase db_user = 'postgres' db_pass = nil - connection = PGconn.connect(db_host, db_port, db_opts, db_tty, - db_name, db_user, db_pass) + connection = PG::Connection.new(db_host, db_port, db_opts, db_tty, + db_name, db_user, db_pass) return connection end @@ -206,9 +206,10 @@ class MailshearsTest < MiniTest::Unit::TestCase plugin_dbuser = cfg.send("#{plugin}_dbuser") plugin_dbpass = cfg.send("#{plugin}_dbpass") - plugin_conn = PGconn.connect(plugin_dbhost, plugin_dbport, plugin_dbopts, - plugin_dbtty, plugin_dbname, plugin_dbuser, - plugin_dbpass) + plugin_conn = PG::Connection.new(plugin_dbhost, plugin_dbport, + plugin_dbopts, plugin_dbtty, + plugin_dbname, plugin_dbuser, + plugin_dbpass) sql = File.open("test/sql/#{plugin}.sql").read() plugin_conn.query(sql)