Overhaul everything to get consistent error reports.

diff --git a/bin/mailshears b/bin/mailshears
index 4d69f8fb67ab2df8a7615e4601b01e8228617a84..2c91f17de15408283de45cf48fad6f79cd95dfc7 100755 (executable)
--- a/bin/mailshears
+++ b/bin/mailshears
@@ -30,16 +30,27 @@ end
 require 'mailshears'
 
 # Since we removed both the executable name and the mode name (if it
 require 'mailshears'
 
 # Since we removed both the executable name and the mode name (if it

-# existed) from ARGV, what remains should be the required
-# arguments.
-if (mode == :prune and (ARGV.length() != 0)) or
-    (mode == :rm and (ARGV.length() < 1))   or
-    (mode == :mv and (ARGV.length() != 2))   then
-  puts "ERROR: missing (or extra) command-line arguments."
+# existed) from ARGV, what remains should be the required arguments.
+# Figure out if we have the wrong number of arguments, and store the
+# associated error message in args_error_message if we do.
+args_error_message = nil
+if mode == :prune and ARGV.length() != 0 then
+  args_error_message = "ERROR: prune mode takes no additional arguments."
+elsif mode == :rm and ARGV.length() < 1 then
+  args_error_message = "ERROR: rm mode takes two or more user arguments."
+elsif mode == :mv and ARGV.length() != 2 then
+  args_error_message = "ERROR: mv mode takes exactly two user arguments."
+end
+
+# If we got the wrong number of arguments, we'll have some error
+# message. Report it and quit.
+if not args_error_message.nil? then
+  STDERR.puts args_error_message

   puts "Usage: #{usage}"
   Kernel.exit(ExitCodes::BAD_COMMAND_LINE)
 end
 
   puts "Usage: #{usage}"
   Kernel.exit(ExitCodes::BAD_COMMAND_LINE)
 end
 

+

 cfg = Configuration.new()
 
 # Load each of the plugins that we'll need.
 cfg = Configuration.new()
 
 # Load each of the plugins that we'll need.


@@ -84,15 +95,32 @@ else
   plugin_module = PrunePlugin
 end
 
   plugin_module = PrunePlugin
 end
 

-

 # Buffer the output so that we can avoid printing the informational
 # header when no plugins produce output.
 require 'stringio'
 output_buffer = StringIO.new()
 $stdout = output_buffer
 
 # Buffer the output so that we can avoid printing the informational
 # header when no plugins produce output.
 require 'stringio'
 output_buffer = StringIO.new()
 $stdout = output_buffer
 

+# Parse the remaining arguments as User/Domain objects. If we get some
+# other argument that isn't one of those, it's an error.
+parsed_args = []
+ARGV.each do |arg|
+  begin
+    u = User.new(arg)
+    parsed_args << u
+  rescue InvalidUserError
+    begin
+      d = Domain.new(arg)
+      parsed_args << d
+    rescue InvalidDomainError
+      STDERR.puts "ERROR: invalid user/domain argument #{arg}"
+      Kernel.exit(ExitCodes::BAD_COMMAND_LINE)
+    end
+  end
+end
+

 begin
 begin

-  plugin_module.run(cfg, *ARGV)
+  plugin_module.run(cfg, *parsed_args)

 ensure
   # Now restore stdout, and print the header plus whatever the plugins
   # produced if they produced anything. If they didn't produce any
 ensure
   # Now restore stdout, and print the header plus whatever the plugins
   # produced if they produced anything. If they didn't produce any

diff --git a/doc/TODO b/doc/TODO
index 8482260c44851ba77b34342752902c5dd3d02df6..3a2ebfe097d8fb1d34a5f90a80443b7a4e86c92f 100644 (file)
--- a/doc/TODO
+++ b/doc/TODO
@@ -1,29 +1,12 @@
-* Error reporting sucks, and when a domain or user doesn't exist we
-  should be able to say so. The describe_domain/user functions
-  should also work better. For plugins that don't implement domains,
-  we can return a (count of?) list of users, or fall back to the
-  user deletion descriptions.
-
-* Error reporting is inconsistent. We know what goes wrong, and then
-  errors bubble up, but where do they get reported? Are they fatal or
-  informational? Do we want later plugins to run of earlier ones
-  failed?
-
-* Implement "mv".
-
-  Design:
-
-  This should only work from a domain that exists in postfixadmin to a
-  domain that already exists in postfixadmin. If the target domain
-  does not exist, we should error out as soon as possible.
-
-  Once we're sure that the target domain does exist, we can hand off
-  the "mv" operation to the plugins. Some of them might have nothing
-  to do -- that's fine.
-
-  MAKE SURE WE DON'T OVERWRITE AN EXISTING USER!
-
-* Add convenience methods (e.g. user_exists, domain_exists) that we
-  can use in the tests to check results.
-
-* Potentially add OpenDKIM support.
+* There is essentially no error handling. We report errors, but we
+  don't fail when we see one. The main reason for this is that we
+  don't know when each plugin will be run. If the first plugin
+  encounters an error, we could quit right there. But what if the
+  third one fails after the first two succeed? We would need some kind
+  of rollback mechanism.
+
+  For "mv", a rollback is conceivable. But with "rm", there's no going
+  back. Maybe relying on the user to interpret the output and go
+  fix stuff himself is the best we can do?
+
+* Add OpenDKIM support.

diff --git a/lib/common/agendav_plugin.rb b/lib/common/agendav_plugin.rb
index ae9b11c0b05d89874ebdc6dedbc2e85af36c6b89..fc19d3d60fd45fccfec195ca745de1c0d7b5b3b5 100644 (file)
--- a/lib/common/agendav_plugin.rb
+++ b/lib/common/agendav_plugin.rb
@@ -1,4 +1,5 @@
 require 'common/plugin'
 require 'common/plugin'

+require 'common/user'

 
 module AgendavPlugin
   # Code that all Agendav plugins (Prune, Rm, Mv...) will
 
 module AgendavPlugin
   # Code that all Agendav plugins (Prune, Rm, Mv...) will


@@ -18,17 +19,11 @@ module AgendavPlugin
 
 
   def describe_domain(domain)
 
 
   def describe_domain(domain)

-    # AgenDAV doesn't have a concept of domains.
-    return domain
+    return domain.to_s()

   end
 
   end
 

-

   def describe_user(user)
   def describe_user(user)

-    if self.user_exists(user)
-      return "Username: #{user}"
-    else
-      return 'User not found'
-    end
+    return user.to_s()

   end
 
 
   end
 
 


@@ -37,7 +32,7 @@ module AgendavPlugin
     # Produce a list of AgenDAV users. This is public because it's
     # useful in testing.
     #
     # Produce a list of AgenDAV users. This is public because it's
     # useful in testing.
     #

-    usernames = []
+    users = []

 
     # Just assume PostgreSQL for now.
     begin
 
     # Just assume PostgreSQL for now.
     begin


@@ -54,7 +49,7 @@ module AgendavPlugin
       sql_query += '(SELECT user_from FROM shared);'
 
       connection.query(sql_query) do |result|
       sql_query += '(SELECT user_from FROM shared);'
 
       connection.query(sql_query) do |result|

-        usernames = result.field_values('username')
+        users = result.field_values('username')

       end
 
       connection.close()
       end
 
       connection.close()


@@ -63,7 +58,7 @@ module AgendavPlugin
       raise DatabaseError.new(e)
     end
 
       raise DatabaseError.new(e)
     end
 

-    return usernames
+    return users.map{ |u| User.new(u) }

   end
 
 end
   end
 
 end

diff --git a/lib/common/davical_plugin.rb b/lib/common/davical_plugin.rb
index 8c9646f2cf95f8fb48198cd1f71927c276856974..557c1dd1b3bf18e012cdf234e6400b05d4de7925 100644 (file)
--- a/lib/common/davical_plugin.rb
+++ b/lib/common/davical_plugin.rb
@@ -1,4 +1,5 @@
 require 'common/plugin'
 require 'common/plugin'

+require 'common/user'

 
 module DavicalPlugin
   # Code that all Davical plugins (Prune, Rm, Mv...) will share.  That
 
 module DavicalPlugin
   # Code that all Davical plugins (Prune, Rm, Mv...) will share.  That


@@ -18,7 +19,7 @@ module DavicalPlugin
 
   def describe_domain(domain)
     # DAViCal doesn't have a concept of domains.
 
   def describe_domain(domain)
     # DAViCal doesn't have a concept of domains.

-    return domain
+    return domain.to_s()

   end
 
 
   end
 
 


@@ -62,7 +63,7 @@ module DavicalPlugin
       raise DatabaseError.new(e)
     end
 
       raise DatabaseError.new(e)
     end
 

-    return usernames
+    return usernames.map{ |u| User.new(u) }

   end
 
 
   end
 
 


@@ -85,7 +86,7 @@ module DavicalPlugin
       sql_query += "      ON principal.user_no = usr.user_no) "
       sql_query += "WHERE usr.username = $1;"
 
       sql_query += "      ON principal.user_no = usr.user_no) "
       sql_query += "WHERE usr.username = $1;"
 

-      connection.query(sql_query, [user]) do |result|
+      connection.query(sql_query, [user.to_s()]) do |result|

         if result.num_tuples > 0
           principal_id = result[0]['principal_id']
         end
         if result.num_tuples > 0
           principal_id = result[0]['principal_id']
         end

diff --git a/lib/common/domain.rb b/lib/common/domain.rb
new file mode 100644 (file)
index 0000000..5fb345b
--- /dev/null
+++ b/lib/common/domain.rb
@@ -0,0 +1,23 @@
+require 'common/errors'
+
+class Domain
+
+  @domain = nil
+
+  def initialize(domain)
+    if domain.empty? then
+      msg = "domain cannot be empty"
+      raise InvalidDomainError.new(msg)
+    end
+
+    @domain = domain
+  end
+
+  def to_s()
+    return @domain
+  end
+
+  def ==(other)
+    return self.to_s() == other.to_s()
+  end
+end

diff --git a/lib/common/dovecot_plugin.rb b/lib/common/dovecot_plugin.rb
index dfd87494c797642d68614ad3ca8e0a0f5a6169ad..4718483bec420e0e11731502597069300c816cb4 100644 (file)
--- a/lib/common/dovecot_plugin.rb
+++ b/lib/common/dovecot_plugin.rb
@@ -1,5 +1,8 @@
-require 'common/plugin'
+require 'common/domain'

 require 'common/filesystem'
 require 'common/filesystem'

+require 'common/plugin'
+require 'common/user'
+

 
 module DovecotPlugin
   # Code that all Dovecot plugins (Prune, Rm, Mv...) will
 
 module DovecotPlugin
   # Code that all Dovecot plugins (Prune, Rm, Mv...) will


@@ -29,55 +32,31 @@ module DovecotPlugin
   end
 
 
   end
 
 

+  def domain_exists(domain)
+    domains = list_domains()
+    return domains.include?(domain)
+  end
+
+

   protected;
 
   def get_domain_path(domain)
     # Return the filesystem path for the given domain.
     # That is, the directory where its mail is stored.
     # Only works if the domain directory exists!
   protected;
 
   def get_domain_path(domain)
     # Return the filesystem path for the given domain.
     # That is, the directory where its mail is stored.
     # Only works if the domain directory exists!

-    domain_path = File.join(@domain_root, domain)
-
-    if File.directory?(domain_path)
-      return domain_path
-    else
-      raise NonexistentDomainError.new(domain)
-    end
+    return File.join(@domain_root, domain.to_s())

   end
 
 
   end
 
 

-  def get_user_path(user, existence_check = true)
-    # Return the filesystem path of this user's mailbox. With
-    # existence_check = true, it only works if the user exists!
-    # We always require that the domain exists.
-    if not user.include?('@')
-      msg = "#{user}: Users must contain an '@' symbol."
-      raise InvalidUserError.new(msg)
-    end
-
-    user_parts = user.split('@')
-    local_part = user_parts[0]
-    domain_part = user_parts[1]
-
-    begin
-      domain_path = get_domain_path(domain_part)
-    rescue NonexistentDomainError
-      raise NonexistentUserError.new(user)
-    end
-
-    user_path = File.join(domain_path, local_part)
-
-    return user_path if not existence_check
-
-    if File.directory?(user_path)
-      return user_path
-    else
-      raise NonexistentUserError.new(user)
-    end
+  def get_user_path(user)
+    # Return the filesystem path of this user's mailbox.
+    domain_path = get_domain_path(user.domain())
+    return File.join(domain_path, user.localpart())

   end
 
 
   def list_domains()
   end
 
 
   def list_domains()

-    return Filesystem.get_subdirs(@domain_root)
+    return Filesystem.get_subdirs(@domain_root).map{ |d| Domain.new(d) }

   end
 
   def list_domains_users(domains)
   end
 
   def list_domains_users(domains)


@@ -92,7 +71,7 @@ module DovecotPlugin
         usernames = Filesystem.get_subdirs(domain_path)
 
         usernames.each do |username|
         usernames = Filesystem.get_subdirs(domain_path)
 
         usernames.each do |username|

-          users << "#{username}@#{domain}"
+          users << User.new("#{username}@#{domain}")

         end
       rescue NonexistentDomainError
         # Party hard.
         end
       rescue NonexistentDomainError
         # Party hard.

diff --git a/lib/common/errors.rb b/lib/common/errors.rb
index bcae691f9c3fbc7405558ddd82ecb3e883f9a8f4..7a2fbba565c035c7584972c3de211b0acf4ce4d2 100644 (file)
--- a/lib/common/errors.rb
+++ b/lib/common/errors.rb
@@ -8,12 +8,20 @@ end
 class InvalidUserError < StandardError
 end
 
 class InvalidUserError < StandardError
 end
 

+# Domain is syntactically invalid.
+class InvalidDomainError < StandardError
+end
+

 
 # Used to indicate that an user does not exist.
 class NonexistentUserError < StandardError
 end
 
 
 # Used to indicate that an user does not exist.
 class NonexistentUserError < StandardError
 end
 

-

 # Used to indicate that a domain does not exist.
 class NonexistentDomainError < StandardError
 end
 # Used to indicate that a domain does not exist.
 class NonexistentDomainError < StandardError
 end

+
+
+# When you try to rename a user on top of one that already exists.
+class UserAlreadyExistsError < StandardError
+end

diff --git a/lib/common/plugin.rb b/lib/common/plugin.rb
index 272f0df56749419e7f5dc14843232544ac5ee0e8..c6f750bc3f4d4c2f20e1c11c31f372e849afd711 100644 (file)
--- a/lib/common/plugin.rb
+++ b/lib/common/plugin.rb
@@ -1,3 +1,6 @@
+require 'common/domain'
+require 'common/user'
+

 # All plugins should include this module. It defines the basic
 # operations that all plugins are supposed to support.
 module Plugin
 # All plugins should include this module. It defines the basic
 # operations that all plugins are supposed to support.
 module Plugin


@@ -26,7 +29,7 @@ module Plugin
     end
 
     def dummy_runner()
     end
 
     def dummy_runner()

-      # The RummyRunner associated with this plugin.
+      # The DummyRunner associated with this plugin.

       raise NotImplementedError
     end
 
       raise NotImplementedError
     end
 


@@ -48,6 +51,18 @@ module Plugin
     end
   end
 
     end
   end
 

+  def describe(target)
+    # A generic version of describe_user/describe_domain that
+    # dispatches base on the class of the target.
+    if target.is_a?(User)
+      return describe_user(target)
+    elsif target.is_a?(Domain)
+      return describe_domain(target)
+    else
+      raise NotImplementedError
+    end
+  end
+

   def describe_domain(domain)
     # Provide a "description" of the domain. This is output along
     # with the domain name and can be anything of use to the system
   def describe_domain(domain)
     # Provide a "description" of the domain. This is output along
     # with the domain name and can be anything of use to the system


@@ -67,27 +82,28 @@ module Plugin
     raise NotImplementedError
   end
 
     raise NotImplementedError
   end
 

-  def user_exists(username)
+  def user_exists(user)

     # Does the given username exist for this plugin? We use a naive
     # implementation here based on list_users() which is required to
     # exist above. Plugins can override this with something fast.
     users = list_users()
     # Does the given username exist for this plugin? We use a naive
     # implementation here based on list_users() which is required to
     # exist above. Plugins can override this with something fast.
     users = list_users()

-    return users.include?(username)
+    return users.include?(user)

   end
 
   def list_domains_users(domains)
   end
 
   def list_domains_users(domains)

-    # Get all usernames belonging to the given domains. If a username
-    # ends in @example.com, it belongs to the domain example.com
+    # Get all users belonging to the given domains. If a user has
+    # domainpart "example.com" then it belongs to the domain
+    # "example.com".

     #
     # This uses a naive loop, but relies only on the existence of a
     # list_users() method which is guaranteed above. More efficient
     # implementations can usually be made within the plugin.
     domains_users = []
 
     #
     # This uses a naive loop, but relies only on the existence of a
     # list_users() method which is guaranteed above. More efficient
     # implementations can usually be made within the plugin.
     domains_users = []
 

-    usernames = list_users();
+    users = list_users();

     domains.each do |d|
     domains.each do |d|

-      matches = usernames.select do |username|
-        username =~ /@#{d}$/
+      matches = users.select do |user|
+        user.domainpart() == d.to_s()

       end
 
       domains_users += matches
       end
 
       domains_users += matches

diff --git a/lib/common/postfixadmin_plugin.rb b/lib/common/postfixadmin_plugin.rb
index 97d59f267214060a7b9824d47338727e29b58b24..60880d9e7bad17cf2298b4b0c9498d1007258039 100644 (file)
--- a/lib/common/postfixadmin_plugin.rb
+++ b/lib/common/postfixadmin_plugin.rb
@@ -1,4 +1,6 @@
+require 'common/domain'

 require 'common/plugin'
 require 'common/plugin'

+require 'common/user'

 require 'pg'
 
 module PostfixadminPlugin
 require 'pg'
 
 module PostfixadminPlugin


@@ -19,13 +21,13 @@ module PostfixadminPlugin
 
   def describe_user(user)
     # There's no other unique identifier in PostfixAdmin
 
   def describe_user(user)
     # There's no other unique identifier in PostfixAdmin

-    return user
+    return user.to_s()

   end
 
 
   def describe_domain(domain)
     # There's no other unique identifier in PostfixAdmin
   end
 
 
   def describe_domain(domain)
     # There's no other unique identifier in PostfixAdmin

-    return domain
+    return domain.to_s()

   end
 
 
   end
 
 


@@ -53,7 +55,7 @@ module PostfixadminPlugin
       raise DatabaseError.new(e)
     end
 
       raise DatabaseError.new(e)
     end
 

-    return domains
+    return domains.map{ |d| Domain.new(d) }

   end
 
 
   end
 
 


@@ -89,7 +91,7 @@ module PostfixadminPlugin
       raise DatabaseError.new(e)
     end
 
       raise DatabaseError.new(e)
     end
 

-    return users
+    return users.map{ |u| User.new(u) }

   end
 
 
   end
 
 


@@ -108,7 +110,7 @@ module PostfixadminPlugin
 
       sql_query  = 'SELECT username FROM mailbox WHERE domain IN $1;'
 
 
       sql_query  = 'SELECT username FROM mailbox WHERE domain IN $1;'
 

-      connection.query(sql_query, [domains]) do |result|
+      connection.query(sql_query, domains.map{|d| d.to_s()}) do |result|

         usernames = result.field_values('username')
       end
 
         usernames = result.field_values('username')
       end
 


@@ -118,7 +120,7 @@ module PostfixadminPlugin
       raise DatabaseError.new(e)
     end
 
       raise DatabaseError.new(e)
     end
 

-    return usernames
+    return usernames.map{ |u| User.new(u) }

   end
 
 
   end
 
 

diff --git a/lib/common/roundcube_plugin.rb b/lib/common/roundcube_plugin.rb
index 1f9805a3e9d079243da75859f6992d7688e6f266..b1c74a6d7494420ea9908389317c9d03e39d64f9 100644 (file)
--- a/lib/common/roundcube_plugin.rb
+++ b/lib/common/roundcube_plugin.rb
@@ -1,4 +1,5 @@
 require 'common/plugin'
 require 'common/plugin'

+require 'common/user'

 
 module RoundcubePlugin
   # Code that all Roundcube plugins (Prune, Rm, Mv...) will share.
 
 module RoundcubePlugin
   # Code that all Roundcube plugins (Prune, Rm, Mv...) will share.


@@ -18,7 +19,7 @@ module RoundcubePlugin
 
   def describe_domain(domain)
     # Roundcube doesn't have a concept of domains.
 
   def describe_domain(domain)
     # Roundcube doesn't have a concept of domains.

-    return domain
+    return domain.to_s()

   end
 
   def describe_user(user)
   end
 
   def describe_user(user)


@@ -58,7 +59,7 @@ module RoundcubePlugin
       raise DatabaseError.new(e)
     end
 
       raise DatabaseError.new(e)
     end
 

-    return usernames
+    return usernames.map{ |u| User.new(u) }

   end
 
   protected;
   end
 
   protected;


@@ -77,7 +78,7 @@ module RoundcubePlugin
 
       sql_query = "SELECT user_id FROM users WHERE username = $1;"
 
 
       sql_query = "SELECT user_id FROM users WHERE username = $1;"
 

-      connection.query(sql_query, [user]) do |result|
+      connection.query(sql_query, [user.to_s()]) do |result|

         if result.num_tuples > 0
           user_id = result[0]['user_id']
         end
         if result.num_tuples > 0
           user_id = result[0]['user_id']
         end

diff --git a/lib/common/user.rb b/lib/common/user.rb
new file mode 100644 (file)
index 0000000..bb239d1
--- /dev/null
+++ b/lib/common/user.rb
@@ -0,0 +1,63 @@
+require 'common/domain'
+require 'common/errors'
+
+class User
+  # A class representing a syntactically valid user; that is, an email
+  # address. Once constructed, you can be sure that it's valid.
+
+  @localpart = nil
+  @domain = nil
+
+  def domain()
+    return @domain
+  end
+
+  def domainpart()
+    return @domain.to_s()
+  end
+
+  def initialize(username)
+    # Initialize this user object, but throw an error if either the
+    # localpart or domainpart are invalid. The one argument is an
+    # email address string.
+    if not username.is_a?(String)
+      msg =  'username must be a String '
+      msg += "but a #{username.class.to_s()} was given"
+      raise InvalidUserError.new(msg)
+    end
+
+    parts = username.split('@')
+
+    if parts.length() < 2 then
+      msg = "the username #{username} does not contain an '@' symbol"
+      raise InvalidUserError.new(msg)
+    end
+
+    localpart = parts[0]
+
+    if localpart.length() > 64 then
+      msg = "the local part of #{username} cannot have more than 64 characters"
+      raise InvalidUserError(msg)
+    end
+
+    if localpart.empty? then
+      msg = "the local part of #{username} cannot be empty"
+      raise InvalidUserError.new(msg)
+    end
+
+    @localpart = localpart
+    @domain = Domain.new(parts[1])
+  end
+
+  def localpart()
+    return @localpart
+  end
+
+  def to_s()
+    return @localpart + '@' + @domain.to_s()
+  end
+
+  def ==(other)
+    return self.to_s() == other.to_s()
+  end
+end

diff --git a/lib/mv/mv_dummy_runner.rb b/lib/mv/mv_dummy_runner.rb
index 11f2ff5c7cf94befb458b6f99433df4f7a056938..662abff73e89d1b176a97b4cea8d780415eec3bf 100644 (file)
--- a/lib/mv/mv_dummy_runner.rb
+++ b/lib/mv/mv_dummy_runner.rb
@@ -4,11 +4,13 @@ class MvDummyRunner
   include Runner
 
   def run(plugin, src, dst)
   include Runner
 
   def run(plugin, src, dst)

-    if src.include?('@') then
-      puts "Would move user: #{src} to #{dst}"
-    else
-      raise NotImplementedError.new('Only users can be moved.')
+
+    if src.is_a?(Domain) or dst.is_a?(Domain) then
+      msg = 'only users can be moved'
+      raise NotImplementedError.new(msg)

     end
     end

+
+    puts "Would move user #{src.to_s()} to #{dst.to_s()}."

   end
 
 end
   end
 
 end

diff --git a/lib/mv/mv_plugin.rb b/lib/mv/mv_plugin.rb
index 29f697c2da028275af40fa3da8f031d5af254592..089e6c772bc7d9ddec80db26359449aae08b2c73 100644 (file)
--- a/lib/mv/mv_plugin.rb
+++ b/lib/mv/mv_plugin.rb
@@ -13,7 +13,7 @@ module MvPlugin
     return MvDummyRunner
   end
 
     return MvDummyRunner
   end
 

-  def mv_user(from, to)
+  def mv_user(src, dst)

     # Rename the given user.
     raise NotImplementedError
   end
     # Rename the given user.
     raise NotImplementedError
   end

diff --git a/lib/mv/mv_runner.rb b/lib/mv/mv_runner.rb
index ee762e1e70cf4198c1630b3404973d20c75ac264..6da6f6d3bb7fd6311d6e27915d5c10d230f981f1 100644 (file)
--- a/lib/mv/mv_runner.rb
+++ b/lib/mv/mv_runner.rb
@@ -1,3 +1,4 @@
+require 'common/domain'

 require 'common/errors'
 require 'common/runner'
 
 require 'common/errors'
 require 'common/runner'
 


@@ -5,35 +6,50 @@ class MvRunner
   include Runner
 
   def run(plugin, src, dst)
   include Runner
 
   def run(plugin, src, dst)

-    # Why think too hard? An user has an @, a domain doesn't.
-    if not src.include?('@') then
-      # We only support moving users right now, and the destination
-      # domain must already exist.
-      raise NotImplementedError.new('Only users can be moved.')
-    end

 
 

-    # Handle this once so we don't have to do it in every plugin.
-    if not dst.include?('@') then
-      msg = "the destination user #{dst} is not valid"
-      raise InvalidUserError.new(msg)
+    if src.is_a?(Domain) or dst.is_a?(Domain) then
+      msg = 'only users can be moved'
+      raise NotImplementedError.new(msg)

     end
 
     begin
     end
 
     begin

-      src_description = plugin.describe_user(src)
+      src_description = plugin.describe(src)

       plugin.mv_user(src, dst)
       plugin.mv_user(src, dst)

-      msg  = "Moved user #{src} (#{src_description}) "
-      msg += "to #{dst}."
+      dst_description = plugin.describe(dst)
+
+      msg  = "Moved user #{src} "
+
+      # Only append the extra description if it's useful.
+      if not src_description.nil? and
+         not src_description.empty? and
+         not src_description == src.to_s() then
+        msg += "(#{src_description}) "
+      end
+
+      msg += "to #{dst}"
+
+      # Only append the extra description if it's useful.
+      if not dst_description.nil? and
+         not dst_description.empty? and
+         not dst_description == dst.to_s() then
+        msg += " (#{dst_description})"
+      end
+
+      msg += "."

       report(plugin, msg)
       report(plugin, msg)

-    rescue InvalidUserError => e
-      report(plugin, e.message)
+

     rescue NonexistentUserError => e
     rescue NonexistentUserError => e

-      report(plugin, e.message)
+      # This means that the SOURCE user didn't exist, since a
+      # nonexistent destination user is perfectly expected.
+      report(plugin, "Source user #{src.to_s()} not found.")

     rescue NonexistentDomainError => e
     rescue NonexistentDomainError => e

-      report(plugin, e.message)
-    rescue StandardError => e
-      msg = "There was an error (#{e.to_s}) moving the user: #{e.message}"
-      report(plugin, msg)
-      Kernel.exit(ExitCodes::DATABASE_ERROR)
+      # This could mean that the source domain doesn't exist, but in
+      # that case, we just report that the source user doesn't
+      # exist. So a nonexistent domain refers to a nonexistent
+      # DESTINATION domain.
+      report(plugin, "Destination domain #{dst.domainpart()} not found.")
+    rescue UserAlreadyExistsError => e
+      report(plugin, "Destination user #{dst.to_s()} already exists.")

     end
   end
 
     end
   end
 

diff --git a/lib/mv/plugins/agendav.rb b/lib/mv/plugins/agendav.rb
index 9c508c9866ca3a37df0ea7ca5d17c603690bc0eb..0324e6dc9e5003141ccf7ec6c8aacda1b3eb7130 100644 (file)
--- a/lib/mv/plugins/agendav.rb
+++ b/lib/mv/plugins/agendav.rb
@@ -8,12 +8,15 @@ class AgendavMv
   include AgendavPlugin
   include MvPlugin
 
   include AgendavPlugin
   include MvPlugin
 

+  def mv_user(src, dst)
+    # It's obviously an error if the source user does not exist. It
+    # would also be an error if the destination domain didn't exist;
+    # however, Agendav doesn't know about domains, so we let that slide.
+    raise NonexistentUserError.new(src.to_s()) if not user_exists(src)

 
 

-  def mv_domain(from, to)
-    # AgenDAV doesn't have a concept of domains.
-  end
+    # And it's an error if the destination user exists already.
+    raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst)

 
 

-  def mv_user(from, to)

     sql_queries = ['UPDATE prefs SET username = $1 WHERE username = $2;']
     sql_queries << 'UPDATE shared SET user_from = $1 WHERE user_from = $2;'
     sql_queries << 'UPDATE shared SET user_which = $1 WHERE user_which = $2;'
     sql_queries = ['UPDATE prefs SET username = $1 WHERE username = $2;']
     sql_queries << 'UPDATE shared SET user_from = $1 WHERE user_from = $2;'
     sql_queries << 'UPDATE shared SET user_which = $1 WHERE user_which = $2;'


@@ -28,7 +31,7 @@ class AgendavMv
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [to, from])
+        connection.query(sql_query, [dst.to_s(), src.to_s()])

       end
 
       connection.close()
       end
 
       connection.close()

diff --git a/lib/mv/plugins/davical.rb b/lib/mv/plugins/davical.rb
index a7cfe80c5337ae3d87f99fd26f3c72bb350239aa..bd66df8bd8d979a3dd6aa74c4fed6f43a658deaf 100644 (file)
--- a/lib/mv/plugins/davical.rb
+++ b/lib/mv/plugins/davical.rb
@@ -12,15 +12,19 @@ class DavicalMv
   include MvPlugin
 
 
   include MvPlugin
 
 

-  def mv_domain(from, to)
-    # DAViCal doesn't have a concept of domains.
-  end
-
-
-  def mv_user(from, to)
+  def mv_user(src, dst)

     # Switch the given usernames. DAViCal uses foreign keys properly
     # and only supports postgres, so we let the ON UPDATE CASCADE
     # trigger handle most of the work.
     # Switch the given usernames. DAViCal uses foreign keys properly
     # and only supports postgres, so we let the ON UPDATE CASCADE
     # trigger handle most of the work.

+
+    # It's obviously an error if the source user does not exist. It
+    # would also be an error if the destination domain didn't exist;
+    # however, DAViCal doesn't know about domains, so we let that slide.
+    raise NonexistentUserError.new(src.to_s()) if not user_exists(src)
+
+    # And it's an error if the destination user exists already.
+    raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst)
+

     sql_queries = ['UPDATE usr SET username = $1 WHERE username = $2']
 
     begin
     sql_queries = ['UPDATE usr SET username = $1 WHERE username = $2']
 
     begin


@@ -33,7 +37,7 @@ class DavicalMv
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [to, from])
+        connection.query(sql_query, [dst.to_s(), src.to_s()])

       end
 
       connection.close()
       end
 
       connection.close()

diff --git a/lib/mv/plugins/dovecot.rb b/lib/mv/plugins/dovecot.rb
index 9cdfc36ead95a013f03cacadd5763c674ea521fc..bd3084edde576d7e9d1eb57dc6a25644f2d4e46d 100644 (file)
--- a/lib/mv/plugins/dovecot.rb
+++ b/lib/mv/plugins/dovecot.rb
@@ -9,36 +9,43 @@ class DovecotMv
   include DovecotPlugin
   include MvPlugin
 
   include DovecotPlugin
   include MvPlugin
 

-  def mv_user(user_from, user_to)
-    # Move the user's mail directory from one domain (that should
-    # exist) to another domain (that should exist).
+  def mv_user(src, dst)
+    # It's obviously an error if the source user does not exist.
+    raise NonexistentUserError.new(src.to_s()) if not user_exists(src)
+
+    # And it's an error if the destination user exists already.
+    raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst)
+
+    # But is it an error if the target domain does not exist? That's a
+    # bit subtle... The domain may exist in the database, but if it
+    # has not received any mail yet, then its directory won't exist
+    # on-disk.
+    #
+    # There are two possible "oops" scenarios resulting from the fact
+    # that we may run either the Postfixadmin move first or the
+    # Dovecot move first. If we move the user in the database, we
+    # definitely want to move him on disk (that is, we should create
+    # the directory here). But if we move him on disk first, then we
+    # don't know if the database move will fail! We don't want to move
+    # his mail files if he won't get moved in the database.

     #
     #

-    # This is actually a little subtle, because none of the
-    # directories in question have to exist. If I create two new
-    # domains in postfixadmin and one mailbox but never deliver any
-    # mail, it should be possible to move the mailbox to the other
-    # domain. No directories will exist until mail is delivered,
-    # though.
-
-    # get_user_path() will fail if there's no mailbox to move. It will
-    # also fail on an invalid user -- oh well!
-    begin
-      from_path = self.get_user_path(user_from)
-    rescue NonexistentUserError
-      # Do nothing, there's no source mailbox...
-      return
+    # Faced with two equally-bad (but easy-to-fix) options, we do the
+    # simplest thing and fail if the destination domain directory
+    # doesn't exist. If nothing else, this is at least consistent.
+    if not self.domain_exists(dst.domain()) then
+      raise NonexistentDomainError.new(dst.domainpart())

     end
 
     # We may need to create the target domain directory, even if the
     # domain exists in the database.
     end
 
     # We may need to create the target domain directory, even if the
     # domain exists in the database.

-    domain_to = user_to.split('@')[1]
-    domain_dir = self.get_domain_path(domain_to)
-    FileUtils.mkdir_p(domain_dir)
-
-    # The from_path exists or else we would have returned earlier. The
-    # parent of to_path exists because we just created it.
-    to_path = self.get_user_path(user_to, false)
-    FileUtils.mv(from_path, to_path)
+    FileUtils.mkdir_p(self.get_domain_path(dst.domain()))
+
+    # The parent of dst_path exists because we just created it.The
+    # source path should exist too, because the "source user" does,
+    # and, well, how did we determine that?
+    src_path = self.get_user_path(src)
+    dst_path = self.get_user_path(dst)
+    FileUtils.mv(src_path, dst_path)

   end
 
 end
   end
 
 end

diff --git a/lib/mv/plugins/postfixadmin.rb b/lib/mv/plugins/postfixadmin.rb
index 3b6d1884975b939bbf7bd01f370fbde10cdf653a..338be052f63affe1bb8deab2041cd994994cfbb6 100644 (file)
--- a/lib/mv/plugins/postfixadmin.rb
+++ b/lib/mv/plugins/postfixadmin.rb
@@ -8,20 +8,19 @@ class PostfixadminMv
   include PostfixadminPlugin
   include MvPlugin
 
   include PostfixadminPlugin
   include MvPlugin
 

-  def mv_user(user_from, user_to)
-    if not user_exists(user_from)
-      raise NonexistentUserError.new("source user #{user_from} does not exist")
-    end
-
-    user_to_parts = user_to.split('@')
-    localpart_to = user_to_parts[0]
-    domain_to = user_to_parts[1]
+  def mv_user(src, dst)
+    # It's obviously an error if the source user does not exist.
+    raise NonexistentUserError.new(src.to_s()) if not user_exists(src)

 
 

-    if not domain_exists(domain_to)
-      msg = "destination domain #{domain_to} does not exist"
-      raise NonexistentDomainError.new(msg)
+    # And if the destination domain does not exist.
+    if not domain_exists(dst.domain())
+      raise NonexistentDomainError.new(dst.domain.to_s())

     end
 
     end
 

+    # But the destination *user* should *not* exist.
+    # And it's an error if the destination user exists already.
+    raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst)
+

     mailbox_query  = 'UPDATE mailbox SET '
     mailbox_query += '  username=$1,'
     mailbox_query += '  domain=$2,'
     mailbox_query  = 'UPDATE mailbox SET '
     mailbox_query += '  username=$1,'
     mailbox_query += '  domain=$2,'


@@ -51,10 +50,10 @@ class PostfixadminMv
 
       sql_queries.each do |sql_query|
           varchar = 1043 # from pg_type.h
 
       sql_queries.each do |sql_query|
           varchar = 1043 # from pg_type.h

-          params = [{:value => user_to, :type => varchar},
-                    {:value => domain_to, :type => varchar},
-                    {:value => localpart_to, :type => varchar},
-                    {:value => user_from, :type => varchar}]
+          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
 
         connection.query(sql_query, params)
       end
 

diff --git a/lib/mv/plugins/roundcube.rb b/lib/mv/plugins/roundcube.rb
index bac8344607d925c26f8652aa9a141ff87eee736c..4b3ebcb5d20084d8c342445d2b5566a98324d6d5 100644 (file)
--- a/lib/mv/plugins/roundcube.rb
+++ b/lib/mv/plugins/roundcube.rb
@@ -9,11 +9,15 @@ class RoundcubeMv
   include MvPlugin
 
 
   include MvPlugin
 
 

-  def mv_domain(from, to)
-    # Roundcube doesn't have a concept of domains.
-  end
+  def mv_user(src, dst)
+    # It's obviously an error if the source user does not exist. It
+    # would also be an error if the destination domain didn't exist;
+    # however, Roundcube doesn't know about domains, so we let that slide.
+    raise NonexistentUserError.new(src.to_s()) if not user_exists(src)
+
+    # And it's an error if the destination user exists already.
+    raise UserAlreadyExistsError.new(dst.to_s()) if user_exists(dst)

 
 

-  def mv_user(from, to)

     sql_queries = ['UPDATE users SET username = $1 WHERE username = $2;']
 
     begin
     sql_queries = ['UPDATE users SET username = $1 WHERE username = $2;']
 
     begin


@@ -26,7 +30,7 @@ class RoundcubeMv
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [to, from])
+        connection.query(sql_query, [dst.to_s(), src.to_s()])

       end
 
       connection.close()
       end
 
       connection.close()

diff --git a/lib/prune/prune_dummy_runner.rb b/lib/prune/prune_dummy_runner.rb
index 6cb789fcee6c4243f7e4a819eaf55fc8f8102e76..64c2606979aad0298cd94b3547d9703da78f21dd 100644 (file)
--- a/lib/prune/prune_dummy_runner.rb
+++ b/lib/prune/prune_dummy_runner.rb
@@ -1,7 +1,6 @@
 require 'common/runner'
 require 'common/runner'

-
-# This is always needed, regardless of which plugin is running.

 require 'prune/plugins/postfixadmin'
 require 'prune/plugins/postfixadmin'

+require 'rm/rm_dummy_runner'

 
 class PruneDummyRunner
   include Runner
 
 class PruneDummyRunner
   include Runner


@@ -16,18 +15,11 @@ class PruneDummyRunner
     db_users = pfa.list_users()
     db_domains = pfa.list_domains()
 
     db_users = pfa.list_users()
     db_domains = pfa.list_domains()
 

-    leftover_users = plugin.get_leftover_users(db_users)
-    leftover_domains = plugin.get_leftover_domains(db_domains)
-
-    leftover_users.each do |user|
-      user_description = plugin.describe_user(user)
-      report(plugin, "Would remove user: #{user} (#{user_description})")
-    end
+    leftovers  = plugin.get_leftover_users(db_users)
+    leftovers += plugin.get_leftover_domains(db_domains)

 
 

-    leftover_domains.each do |domain|
-      domain_description = plugin.describe_domain(domain)
-      report(plugin, "Would remove domain: #{domain} (#{domain_description})")
-    end
+    rm_dummy_runner = RmDummyRunner.new()
+    rm_dummy_runner.run(plugin, *leftovers)

   end
 
 end
   end
 
 end

diff --git a/lib/prune/prune_runner.rb b/lib/prune/prune_runner.rb
index 12a02d7417500917557f6235f5d2c45244931077..8ae8ad218bcbc402e167509dca190ab62cbec5fb 100644 (file)
--- a/lib/prune/prune_runner.rb
+++ b/lib/prune/prune_runner.rb
@@ -1,7 +1,6 @@
 require 'common/runner'
 require 'common/runner'

-
-# This is always needed, regardless of which plugin is running.

 require 'prune/plugins/postfixadmin'
 require 'prune/plugins/postfixadmin'

+require 'rm/rm_runner'

 
 class PruneRunner
   include Runner
 
 class PruneRunner
   include Runner


@@ -16,21 +15,11 @@ class PruneRunner
     db_users = pfa.list_users()
     db_domains = pfa.list_domains()
 
     db_users = pfa.list_users()
     db_domains = pfa.list_domains()
 

-    leftover_users = plugin.get_leftover_users(db_users)
-    leftover_domains = plugin.get_leftover_domains(db_domains)
-
-    leftover_users.each do |user|
-      user_description = plugin.describe_user(user)
-      plugin.delete_user(user)
-      report(plugin, "Removed user: #{user} (#{user_description})")
-    end
-
-    leftover_domains.each do |domain|
-      domain_description = plugin.describe_domain(domain)
-      plugin.delete_domain(domain)
-      report(plugin, "Removed domain: #{domain} (#{domain_description})")
-    end
+    leftovers  = plugin.get_leftover_users(db_users)
+    leftovers += plugin.get_leftover_domains(db_domains)

 
 

+    rm_runner = RmRunner.new()
+    rm_runner.run(plugin, *leftovers)

   end
 
 end
   end
 
 end

diff --git a/lib/rm/plugins/agendav.rb b/lib/rm/plugins/agendav.rb
index 0f4e790f6f4007f124eed570db7d52651f767d8a..bebab618b06f190e979fb79c632c7239a409cccf 100644 (file)
--- a/lib/rm/plugins/agendav.rb
+++ b/lib/rm/plugins/agendav.rb
@@ -12,7 +12,7 @@ class AgendavRm
   def delete_user(user)
     # Delete the given username and any records in other tables
     # belonging to it.
   def delete_user(user)
     # Delete the given username and any records in other tables
     # belonging to it.

-    raise NonexistentUserError.new(user) if not user_exists(user)
+    raise NonexistentUserError.new(user.to_s()) if not user_exists(user)

 
     sql_queries = ['DELETE FROM prefs WHERE username = $1;']
     sql_queries << 'DELETE FROM shared WHERE user_from = $1;'
 
     sql_queries = ['DELETE FROM prefs WHERE username = $1;']
     sql_queries << 'DELETE FROM shared WHERE user_from = $1;'


@@ -27,7 +27,7 @@ class AgendavRm
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [user])
+        connection.query(sql_query, [user.to_s()])

       end
 
       connection.close()
       end
 
       connection.close()

diff --git a/lib/rm/plugins/davical.rb b/lib/rm/plugins/davical.rb
index f7d116e0e783a08364701c97334668d04ca14d45..3526dcff2fa4f9fc57950c32ac50e34c44d6872c 100644 (file)
--- a/lib/rm/plugins/davical.rb
+++ b/lib/rm/plugins/davical.rb
@@ -16,7 +16,7 @@ class DavicalRm
     # Delete the given username. DAViCal uses foreign keys properly
     # and only supports postgres, so we let the ON DELETE CASCADE
     # trigger handle most of the work.
     # Delete the given username. DAViCal uses foreign keys properly
     # and only supports postgres, so we let the ON DELETE CASCADE
     # trigger handle most of the work.

-    raise NonexistentUserError.new(user) if not user_exists(user)
+    raise NonexistentUserError.new(user.to_s()) if not user_exists(user)

 
     sql_queries = ['DELETE FROM usr WHERE username = $1']
 
 
     sql_queries = ['DELETE FROM usr WHERE username = $1']
 


@@ -30,7 +30,7 @@ class DavicalRm
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [user])
+        connection.query(sql_query, [user.to_s()])

       end
 
       connection.close()
       end
 
       connection.close()

diff --git a/lib/rm/plugins/dovecot.rb b/lib/rm/plugins/dovecot.rb
index 837a7fd68b4da94c3c8363d64035622c410796be..0ced5bec3acf4aca70bbdd7008882112c78ba412 100644 (file)
--- a/lib/rm/plugins/dovecot.rb
+++ b/lib/rm/plugins/dovecot.rb
@@ -11,14 +11,22 @@ class DovecotRm
 
 
   def delete_domain(domain)
 
 
   def delete_domain(domain)

-    # Will raise an exception if the path doesn't exist.

     domain_path = self.get_domain_path(domain)
     domain_path = self.get_domain_path(domain)

+
+    if not File.directory?(domain_path)
+      raise NonexistentDomainError.new(domain.to_s())
+    end
+

     FileUtils.rm_r(domain_path)
   end
 
   def delete_user(user)
     FileUtils.rm_r(domain_path)
   end
 
   def delete_user(user)

-    # Will raise an exception if the path doesn't exist.

     user_path = self.get_user_path(user)
     user_path = self.get_user_path(user)

+
+    if not File.directory?(user_path)
+      raise NonexistentUserError.new(user.to_s())
+    end
+

     FileUtils.rm_r(user_path)
   end
 
     FileUtils.rm_r(user_path)
   end
 

diff --git a/lib/rm/plugins/postfixadmin.rb b/lib/rm/plugins/postfixadmin.rb
index 94526bcd25bfc058a1c05bd92081caeb16fa5caa..eddf93976981a5d031772cb3398001346d0fbc12 100644 (file)
--- a/lib/rm/plugins/postfixadmin.rb
+++ b/lib/rm/plugins/postfixadmin.rb
@@ -10,7 +10,7 @@ class PostfixadminRm
 
 
   def delete_user(user)
 
 
   def delete_user(user)

-    raise NonexistentUserError.new(user) if not user_exists(user)
+    raise NonexistentUserError.new(user.to_s()) if not user_exists(user)

 
     sql_queries = ['DELETE FROM alias WHERE address = $1;']
     # Wipe out any aliases pointed at our user.
 
     sql_queries = ['DELETE FROM alias WHERE address = $1;']
     # Wipe out any aliases pointed at our user.


@@ -33,7 +33,7 @@ class PostfixadminRm
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [user])
+        connection.query(sql_query, [user.to_s()])

       end
 
       # The earlier alias update query will leave things like
       end
 
       # The earlier alias update query will leave things like


@@ -53,7 +53,7 @@ class PostfixadminRm
 
 
   def delete_domain(domain)
 
 
   def delete_domain(domain)

-    raise NonexistentDomainError.new(domain) if not domain_exists(domain)
+    raise NonexistentDomainError.new(domain.to_s()) if not domain_exists(domain)

 
     sql_queries = ['DELETE FROM domain_admins WHERE domain = $1;']
     sql_queries << 'DELETE FROM alias WHERE domain = $1;'
 
     sql_queries = ['DELETE FROM domain_admins WHERE domain = $1;']
     sql_queries << 'DELETE FROM alias WHERE domain = $1;'


@@ -73,7 +73,7 @@ class PostfixadminRm
                                   @db_pass)
 
       sql_queries.each do |sql_query|
                                   @db_pass)
 
       sql_queries.each do |sql_query|

-        connection.query(sql_query, [domain])
+        connection.query(sql_query, [domain.to_s()])

       end
 
       connection.close()
       end
 
       connection.close()


@@ -102,7 +102,7 @@ class PostfixadminRm
                                   @db_pass)
 
       sql_query = 'SELECT COUNT(domain) as count FROM domain WHERE domain = $1;'
                                   @db_pass)
 
       sql_query = 'SELECT COUNT(domain) as count FROM domain WHERE domain = $1;'

-      connection.query(sql_query, [domain]) do |result|
+      connection.query(sql_query, [domain.to_s()]) do |result|

         return false if result.ntuples() < 1
         begin
           count = result.getvalue(0,0).to_i()
         return false if result.ntuples() < 1
         begin
           count = result.getvalue(0,0).to_i()

diff --git a/lib/rm/plugins/roundcube.rb b/lib/rm/plugins/roundcube.rb
index 23782d20a575471820aa279f7ce47b346e68f2e2..a315e77f49b2ec0c9b17c09d228ee55db8fa8459 100644 (file)
--- a/lib/rm/plugins/roundcube.rb
+++ b/lib/rm/plugins/roundcube.rb
@@ -11,7 +11,7 @@ class RoundcubeRm
   def delete_user(user)
     # Delete the given username and any records in other tables
     # belonging to it.
   def delete_user(user)
     # Delete the given username and any records in other tables
     # belonging to it.

-    raise NonexistentUserError.new(user) if not user_exists(user)
+    raise NonexistentUserError.new(user.to_s()) if not user_exists(user)

 
     user_id = self.get_user_id(user)
 
 
     user_id = self.get_user_id(user)
 

diff --git a/lib/rm/rm_dummy_runner.rb b/lib/rm/rm_dummy_runner.rb
index 477608f8e77318e535ef671eca0bb7a629a5d0ad..2ab25e80a3e402f5ba3c21214b30a768d6c2cd2c 100644 (file)
--- a/lib/rm/rm_dummy_runner.rb
+++ b/lib/rm/rm_dummy_runner.rb
@@ -5,13 +5,18 @@ class RmDummyRunner
 
   def run(plugin, *targets)
     targets.each do |target|
 
   def run(plugin, *targets)
     targets.each do |target|

-      if target.include?('@') then
-        user_description = plugin.describe_user(target)
-        report(plugin, "Would remove user: #{target} (#{user_description})")
-      else
-        domain_description = plugin.describe_domain(target)
-        report(plugin, "Would remove domain: #{target} (#{domain_description})")
+      target_description = plugin.describe(target)
+      msg =  "Would remove #{target.class.to_s().downcase()} #{target}"
+
+      # Only append the extra description if it's useful.
+      if not target_description.nil? and
+         not target_description.empty? and
+         not target_description == target.to_s() then
+        msg += " (#{target_description})"

       end
       end

+      msg += '.'
+
+      report(plugin, msg)

     end
 
   end
     end
 
   end

diff --git a/lib/rm/rm_plugin.rb b/lib/rm/rm_plugin.rb
index fe938b128d200b1a10427cbff7cdde17c57bd4b8..fd0c541a44cb25ed7ffbe7e96db3c19b59426265 100644 (file)
--- a/lib/rm/rm_plugin.rb
+++ b/lib/rm/rm_plugin.rb
@@ -15,15 +15,27 @@ module RmPlugin
     return RmDummyRunner
   end
 
     return RmDummyRunner
   end
 

+  def delete(target)
+    # A generic version of delete_user/delete_domain that
+    # dispatches base on the class of the target.
+    if target.is_a?(User)
+      return delete_user(target)
+    elsif target.is_a?(Domain)
+      return delete_domain(target)
+    else
+      raise NotImplementedError
+    end
+  end
+

   def delete_domain(domain)
     # Delete the given domain. Some plugins don't have a concept of
     # domains, so just delete all users with a username that looks
     # like it's in the given domain.
   def delete_domain(domain)
     # Delete the given domain. Some plugins don't have a concept of
     # domains, so just delete all users with a username that looks
     # like it's in the given domain.

-    usernames = list_domains_users([domain])
+    users = list_domains_users([domain])

 
 

-    raise NonexistentDomainError.new(domain) if usernames.empty?
+    raise NonexistentDomainError.new(domain.to_s()) if users.empty?

 
 

-    usernames.each do |u|
+    users.each do |u|

       delete_user(u)
     end
   end
       delete_user(u)
     end
   end

diff --git a/lib/rm/rm_runner.rb b/lib/rm/rm_runner.rb
index 71421547e7e88e02247070680dce940f890c30f2..2df8410c5ea9d6d3d65d2565ec9cc00552d7b1f0 100644 (file)
--- a/lib/rm/rm_runner.rb
+++ b/lib/rm/rm_runner.rb
@@ -6,37 +6,32 @@ class RmRunner
 
   def run(plugin, *targets)
     targets.each do |target|
 
   def run(plugin, *targets)
     targets.each do |target|

-      # Why think too hard? An user has an @, a domain doesn't.
-      if target.include?('@') then
-        begin
-          user_description = plugin.describe_user(target)
-          plugin.delete_user(target)
-          report(plugin, "Removed user: #{target} (#{user_description})")
+      remove_target(plugin, target)
+    end
+  end
+
+  private;
+
+  def remove_target(plugin, target)
+    # Wrap the "remove this thing" operation so that it can be reused
+    # in the prine plugin.
+    target_description = plugin.describe(target)

 
 

-        rescue NonexistentUserError => e
-          report(plugin, "User not found: #{e.to_s}")
-        rescue StandardError => e
-          report(plugin, "There was an error removing the user: #{e.to_s}")
-          Kernel.exit(ExitCodes::DATABASE_ERROR)
-        end
-      else
-        begin
-          domain_description = plugin.describe_domain(target)
-          plugin.delete_domain(target)
-          report(plugin, "Removed domain: #{target} (#{domain_description})")
+    begin
+      plugin.delete(target)
+      msg =  "Removed #{target.class.to_s().downcase()} #{target}"

 
 

-        rescue NonexistentUserError => e
-          # Can happen in the usernames.each... block.
-          report(plugin, "User not found: #{e.to_s}")
-        rescue NonexistentDomainError => e
-          report(plugin, "Domain not found: #{e.to_s}")
-        rescue StandardError => e
-          report(plugin, "There was an error removing the domain: #{e.to_s}")
-          Kernel.exit(ExitCodes::DATABASE_ERROR)
-        end
+      # Only append the extra description if it's useful.
+      if not target_description.nil? and
+         not target_description.empty? and
+         not target_description == target.to_s() then
+        msg += " (#{target_description})"

       end
       end

-    end
+      msg += '.'

 
 

+      report(plugin, msg)
+    rescue NonexistentDomainError, NonexistentUserError => e
+      report(plugin, "#{target.class.to_s()} #{e.to_s} not found.")
+    end

   end
   end

-

 end
 end