Fix Postfixadmin alias updating (don't leave commas at the start/end).

diff --git a/lib/rm/plugins/postfixadmin.rb b/lib/rm/plugins/postfixadmin.rb
index c8e44abd2d9140b5a75d969078e31b6eb836aa02..46eda4736878c25c35e435aee446a89d77be5fe3 100644 (file)
--- 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 96a6e35e6c6ccf5343adca27d4a806031ce0ba33..e46ca11ff4d144c8324030ec170efffe5d87ae17 100644 (file)
--- 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 2ca6d594b0bbc9e2c36cda94858d994a62ba037b..922c91217bfbbb6356feca2f5cc1a12cc4d53f2e 100644 (file)
--- 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 f91c4bf79c8583e36ed570fcebad6005f45971cf..7ded82016072a526874f0c68bc172738c69d3b71 100644 (file)
--- 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 d32fcae00958adc124e4f20885bdfa6bb22e5765..35c1a5c6941d391c008807df8d79fc392ff76183 100644 (file)
--- 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 1d424619b76561c054ccd34f86679ab672d19036..210ca5daa8cfb2485a1bed7a9ec8d00d415d12f0 100644 (file)
--- 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()