From a2d3dd9ee4838fb99557718b4bcdc11c8d1372fd Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Sun, 8 Nov 2015 15:01:16 -0500 Subject: [PATCH] Fix Postfixadmin alias updating (don't leave commas at the start/end). --- lib/rm/plugins/postfixadmin.rb | 25 +++++++++++----------- test/mailshears_test.rb | 33 ++++++++++++++++-------------- test/sql/postfixadmin-fixtures.sql | 10 ++++++--- test/test_mv.rb | 30 ++++++++++++++++----------- test/test_prune.rb | 22 ++++++++++++-------- test/test_rm.rb | 22 ++++++++++++-------- 6 files changed, 82 insertions(+), 60 deletions(-) diff --git a/lib/rm/plugins/postfixadmin.rb b/lib/rm/plugins/postfixadmin.rb index c8e44ab..46eda47 100644 --- a/lib/rm/plugins/postfixadmin.rb +++ b/lib/rm/plugins/postfixadmin.rb @@ -31,10 +31,16 @@ class PostfixadminRm # But aliases don't need to point to a single user! If our user # was part of a multi-recipient alias, we want to remove our user - # from the alias and leave the other recipients. If you're - # wondering about the leftover double-commas, look towards the end - # of the function. - sql_queries << "UPDATE alias SET goto=REPLACE(goto, $1, '');" + # from the alias and leave the other recipients. + # + # We want to delete the comma that precedes/follows the address, + # too. Since the address to be replaced can appear at either the + # beginning or the end of the list (as well as in the middle), we + # have to try to fix both cases: comma before, and comma after. + comma_before = "CONCAT(',', $1)" + comma_after = "CONCAT($1, ',')" + sql_queries << "UPDATE alias SET goto=REPLACE(goto, #{comma_before}, '');" + sql_queries << "UPDATE alias SET goto=REPLACE(goto, #{comma_after}, '');" sql_queries << 'DELETE FROM mailbox WHERE username = $1;' sql_queries << 'DELETE FROM quota WHERE username = $1;' @@ -48,15 +54,10 @@ class PostfixadminRm begin sql_queries.each do |sql_query| - connection.query(sql_query, [user.to_s()]) + varchar = 1043 # from pg_type.h + params = [{:value => user.to_s(), :type => varchar}] + connection.query(sql_query, params) 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() diff --git a/test/mailshears_test.rb b/test/mailshears_test.rb index 96a6e35..e46ca11 100644 --- a/test/mailshears_test.rb +++ b/test/mailshears_test.rb @@ -138,21 +138,24 @@ class MailshearsTest < MiniTest::Unit::TestCase # +-------------------+-------------+------------+ # # - # +------------------------------------------------------+ - # | alias | - # +-------------------+-------------------+--------------+ - # | address | goto | domain | - # +-------------------+-------------------+--------------+ - # | alice@example.com | alice@example.com | example.com | - # +-------------------+-------------------+--------------+ - # | bob@example.com | bob@example.com | example.com | - # +-------------------+-------------------+--------------+ - # | adam@example.net | adam@example.net | example.net | - # +-------------------+-------------------+--------------+ - # | beth@example.net | beth@example.net | example.net | - # +-------------------+-------------------+--------------+ - # | carol@example.net | carol@example.net | example.net | - # +-------------------+-------------------+--------------+ + # +-------------------------------------------------------+ + # | alias | + # +-------------------+--------------------+--------------+ + # | address | goto | domain | + # +-------------------+--------------------+--------------+ + # | alice@example.com | alice@example.com, | example.com | + # | | adam@example.net, | | + # | | bob@example.com, | | + # | | carol@example.net | | + # +-------------------+--------------------+--------------+ + # | bob@example.com | bob@example.com | example.com | + # +-------------------+--------------------+--------------+ + # | adam@example.net | adam@example.net | example.net | + # +-------------------+--------------------+--------------+ + # | beth@example.net | beth@example.net | example.net | + # +-------------------+--------------------+--------------+ + # | carol@example.net | carol@example.net | example.net | + # +-------------------+--------------------+--------------+ # # # +---------------------------------+ diff --git a/test/sql/postfixadmin-fixtures.sql b/test/sql/postfixadmin-fixtures.sql index 2ca6d59..922c912 100644 --- a/test/sql/postfixadmin-fixtures.sql +++ b/test/sql/postfixadmin-fixtures.sql @@ -24,11 +24,8 @@ INSERT INTO mailbox (username, domain, local_part) VALUES ('carol@example.net', 'example.net', 'carol'); /* Each mailbox has an alias pointing to (at least) itself. */ -INSERT INTO alias (address, goto, domain) - VALUES ('alice@example.com', 'alice@example.com', 'example.com'); INSERT INTO alias (address, goto, domain) VALUES ('bob@example.com', 'bob@example.com', 'example.com'); - INSERT INTO alias (address, goto, domain) VALUES ('adam@example.net', 'adam@example.net', 'example.net'); INSERT INTO alias (address, goto, domain) @@ -36,6 +33,13 @@ INSERT INTO alias (address, goto, domain) INSERT INTO alias (address, goto, domain) VALUES ('carol@example.net', 'carol@example.net', 'example.net'); +/* Alice is aliased to some other people, too. */ +INSERT INTO alias (address, goto, domain) + VALUES + ('alice@example.com', + 'alice@example.com,adam@example.net,bob@example.com,carol@example.net', + 'example.com'); + /* Create a domain admin for both example.com and example.net. */ INSERT INTO domain_admins (username, domain) VALUES ('admin@example.com', 'example.com'); diff --git a/test/test_mv.rb b/test/test_mv.rb index f91c4bf..7ded820 100644 --- a/test/test_mv.rb +++ b/test/test_mv.rb @@ -66,14 +66,17 @@ class TestMv < MailshearsTest assert_equal(expected.sort(), actual.sort()) actual = pfamv.list_aliases() - expected = [{'address' => 'alice@example.net', - 'goto' => 'alice@example.net'}, - {'address' => 'bob@example.com', - 'goto' => 'bob@example.com'}, - {'address' => 'adam@example.net', + expected = [{'address' => 'adam@example.net', 'goto' => 'adam@example.net'}, + {'address' => 'alice@example.net', + 'goto' => 'alice@example.net,' + + 'adam@example.net,' + + 'bob@example.com,' + + 'carol@example.net'}, {'address' => 'beth@example.net', - 'goto' => 'beth@example.net'}, + 'goto' => 'beth@example.net'}, + {'address' => 'bob@example.com', + 'goto' => 'bob@example.com'}, {'address' => 'carol@example.net', 'goto' => 'carol@example.net'}] expected.each { |e| assert(actual.include?(e)) } # can't sort dicts @@ -138,14 +141,17 @@ class TestMv < MailshearsTest assert_equal(expected.sort(), actual.sort()) actual = pfamv.list_aliases() - expected = [{'address' => 'alice@example.com', - 'goto' => 'alice@example.com'}, - {'address' => 'bob@example.com', - 'goto' => 'bob@example.com'}, - {'address' => 'adam@example.net', + expected = [{'address' => 'adam@example.net', 'goto' => 'adam@example.net'}, + {'address' => 'alice@example.com', + 'goto' => 'alice@example.com,' + + 'adam@example.net,' + + 'bob@example.com,' + + 'carol@example.net'}, {'address' => 'beth@example.net', - 'goto' => 'beth@example.net'}, + 'goto' => 'beth@example.net'}, + {'address' => 'bob@example.com', + 'goto' => 'bob@example.com'}, {'address' => 'carol@example.net', 'goto' => 'carol@example.net'}] expected.each { |e| assert(actual.include?(e)) } # can't sort dicts diff --git a/test/test_prune.rb b/test/test_prune.rb index d32fcae..35c1a5c 100644 --- a/test/test_prune.rb +++ b/test/test_prune.rb @@ -56,17 +56,21 @@ class TestPrune < MailshearsTest assert_equal(expected, actual) actual = pfapr.list_aliases() - expected = [{'address' => 'alice@example.com', - 'goto' => 'alice@example.com'}, - {'address' => 'bob@example.com', - 'goto' => 'bob@example.com'}, - {'address' => 'adam@example.net', - 'goto' => 'adam@example.net'}, + expected = [{'address' => 'adam@example.net', + 'goto' => 'adam@example.net'}, + {'address' => 'alice@example.com', + 'goto' => 'alice@example.com,' + + 'adam@example.net,' + + 'bob@example.com,' + + 'carol@example.net'}, {'address' => 'beth@example.net', - 'goto' => 'beth@example.net'}, + 'goto' => 'beth@example.net'}, + {'address' => 'bob@example.com', + 'goto' => 'bob@example.com'}, {'address' => 'carol@example.net', - 'goto' => 'carol@example.net'}] - assert_equal(expected, actual) + 'goto' => 'carol@example.net'}] + expected.each { |e| assert(actual.include?(e)) } # can't sort dicts + actual.each { |a| assert(expected.include?(a)) } # can't sort dicts rpr = RoundcubePrune.new(cfg) actual = rpr.list_users() diff --git a/test/test_rm.rb b/test/test_rm.rb index 1d42461..210ca5d 100644 --- a/test/test_rm.rb +++ b/test/test_rm.rb @@ -62,14 +62,17 @@ class TestRm < MailshearsTest actual = pfarm.list_aliases() expected = [{'address' => 'alice@example.com', - 'goto' => 'alice@example.com'}, - {'address' => 'bob@example.com', - 'goto' => 'bob@example.com'}, + 'goto' => 'alice@example.com,' + + 'bob@example.com,' + + 'carol@example.net'}, {'address' => 'beth@example.net', - 'goto' => 'beth@example.net'}, + 'goto' => 'beth@example.net'}, + {'address' => 'bob@example.com', + 'goto' => 'bob@example.com'}, {'address' => 'carol@example.net', - 'goto' => 'carol@example.net'}] - assert_equal(expected, actual) + 'goto' => 'carol@example.net'}] + expected.each { |e| assert(actual.include?(e)) } # can't sort dicts + actual.each { |a| assert(expected.include?(a)) } # can't sort dicts rrm = RoundcubeRm.new(cfg) actual = rrm.list_users() @@ -130,10 +133,11 @@ class TestRm < MailshearsTest actual = pfarm.list_aliases() expected = [{'address' => 'alice@example.com', - 'goto' => 'alice@example.com'}, + 'goto' => 'alice@example.com,bob@example.com'}, {'address' => 'bob@example.com', - 'goto' => 'bob@example.com'}] - assert_equal(expected, actual) + 'goto' => 'bob@example.com'}] + expected.each { |e| assert(actual.include?(e)) } # can't sort dicts + actual.each { |a| assert(expected.include?(a)) } # can't sort dicts rrm = RoundcubeRm.new(cfg) actual = rrm.list_users() -- 2.44.2