email: audit: improve the speed of post-installation setup

- New class: `MainCfDiagnosis`
  - "Advise and repair" mechanism reduces the number of postconf calls

- File locking: lock acquisition moved into audit module
  - Enables finer-grained control

plinth/modules/email_server/__init__.py

@@ -96,6 +96,7 @@ class EmailServerApp(plinth.app.App):
         results = super().diagnose()
         results.extend([r.summarize() for r in audit.domain.get()])
         results.extend([r.summarize() for r in audit.ldap.get()])
+        results.extend([r.summarize() for r in audit.spam.get()])
         return results
 
 

plinth/modules/email_server/audit/domain.py

@@ -6,7 +6,7 @@ from . import models
 
 def get():
     # Stub
-    return [models.Result('Email domains')]
+    return [models.Diagnosis('Email domains')]
 
 
 def repair():

plinth/modules/email_server/audit/ldap.py

@@ -49,12 +49,10 @@ def get():
     Recommended endpoint name:
     GET /audit/ldap
     """
-    results = models.Result('Postfix uses Dovecot for SASL authentication')
+    results = []
-    current_config = postconf.get_many(list(default_config.keys()))
+    with postconf.postconf_mutex.lock_all():
-    for key, value in default_config.items():
+        results.append(check_sasl())
-        if current_config[key] != value:
+    return results
-            results.fails.append('{} should equal {}'.format(key, value))
-    return [results]
 
 
 def repair():
@@ -63,22 +61,35 @@ def repair():
     Recommended endpoint name:
     POST /audit/ldap/repair
     """
-    logger.debug('Updating postconf: %r', default_config)
     actions.superuser_run('email_server', ['-i', 'ldap', 'set_sasl'])
-
-    logger.debug('Setting up postfix services:\n    %r\n    %r',
-                 default_submission_options, default_smtps_options)
     actions.superuser_run('email_server', ['-i', 'ldap', 'set_submission'])
 
 
+def check_sasl():
+    diagnosis = models.MainCfDiagnosis('Postfix-Dovecot SASL integration')
+    current = postconf.get_many_unsafe(default_config.keys())
+    diagnosis.compare_and_advise(current=current, default=default_config)
+    return diagnosis
+
+
+def fix_sasl(diagnosis):
+    diagnosis.assert_resolved()
+    logger.info('Setting postconf: %r', diagnosis.advice)
+    postconf.set_many_unsafe(diagnosis.advice)
+
+
 def action_set_sasl():
-    """Called by email_server ipc set_sasl"""
+    """Called by email_server ipc ldap set_sasl"""
-    postconf.set_many(default_config)
+    with postconf.postconf_mutex.lock_all():
+        fix_sasl(check_sasl())
 
 
 def action_set_submission():
     """Called by email_server ipc set_submission"""
+    logger.info('Set postfix service: %r', default_submission_options)
     postconf.set_master_cf_options(service_flags=submission_flags,
                                    options=default_submission_options)
+
+    logger.info('Set postfix service: %r', default_smtps_options)
     postconf.set_master_cf_options(service_flags=smtps_flags,
                                    options=default_smtps_options)

plinth/modules/email_server/audit/models.py

@@ -5,12 +5,30 @@ import logging
 logger = logging.getLogger(__name__)
 
 
-class Result:
+class UnresolvedIssueError(AssertionError):
+    pass
+
+
+class Diagnosis:
     def __init__(self, title):
         self.title = title
         self.fails = []
         self.errors = []
 
+    def critical(self, message_fmt, *args):
+        """Append a message to the fails list"""
+        if args:
+            self.fails.append(message_fmt % args)
+        else:
+            self.fails.append(message_fmt)
+
+    def error(self, message_fmt, *args):
+        """Append a message to the errors list"""
+        if args:
+            self.errors.append(message_fmt % args)
+        else:
+            self.errors.append(message_fmt)
+
     def summarize(self, log=True):
         """Return a 2-element list for the diagnose function in AppView"""
         if log:
@@ -30,3 +48,34 @@ class Result:
             logger.critical(message)
         for message in self.fails:
             logger.error(message)
+
+
+class MainCfDiagnosis(Diagnosis):
+    def __init__(self, title):
+        super().__init__(title)
+        self.advice = {}
+        self.user = {}
+
+    def flag(self, key, corrected_value=None, user=None):
+        self.advice[key] = corrected_value
+        self.user[key] = user
+
+    def unresolved_issues(self):
+        """Returns an interator of dictionary keys"""
+        for key, value in self.advice.items():
+            if value is None:
+                yield key
+
+    def compare_and_advise(self, current, default):
+        if len(current) > len(default):
+            raise ValueError('Sanity check failed: dictionary sizes')
+        for key, value in default.items():
+            if current.get(key, None) != value:
+                self.flag(key, corrected_value=value)
+                self.critical('%s must equal %s', key, value)
+
+    def assert_resolved(self):
+        """Raises an UnresolvedIssueError if the diagnosis report contains an
+        unresolved issue"""
+        if None in self.advice.values():
+            raise UnresolvedIssueError('Assertion failed')

plinth/modules/email_server/audit/spam.py

@@ -6,21 +6,47 @@ import logging
 from plinth import actions
 
 import plinth.modules.email_server.postconf as postconf
+from . import models
 
 milter_config = {
-    'milter_mail_macros': 'i {auth_type} {auth_authen} {auth_author} '\
+    'milter_mail_macros': 'i ' + ' '.join([
-    '{client_addr} {client_name} {mail_addr} {mail_host} {mail_mailer}',
+        '{auth_type}', '{auth_authen}', '{auth_author}',
+        '{client_addr}', '{client_name}',
+        '{mail_addr}', '{mail_host}', '{mail_mailer}'
+    ]),
+    # XXX In postconf this field is a list
     'smtpd_milters': 'inet:127.0.0.1:11332',
+    # XXX In postconf this field is a list
     'non_smtpd_milters': 'inet:127.0.0.1:11332'
 }
 
 logger = logging.getLogger(__name__)
 
 
+def get():
+    results = []
+    with postconf.postconf_mutex.lock_all():
+        results.append(check_filter())
+    return results
+
+
 def repair():
-    logger.debug('Updating postconf: %r', milter_config)
     actions.superuser_run('email_server', ['-i', 'spam', 'set_filter'])
 
 
+def check_filter():
+    diagnosis = models.MainCfDiagnosis('Postfix milter')
+    current = postconf.get_many_unsafe(milter_config.keys())
+    diagnosis.compare_and_advise(current=current, default=milter_config)
+    return diagnosis
+
+
+def fix_filter(diagnosis):
+    diagnosis.assert_resolved()
+    logger.info('Setting postconf: %r', diagnosis.advice)
+    postconf.set_many_unsafe(diagnosis.advice)
+
+
 def action_set_filter():
-    postconf.set_many(milter_config)
+    with postconf.postconf_mutex.lock_all():
+        fix_filter(check_filter())

plinth/modules/email_server/postconf.py

@@ -29,13 +29,17 @@ class ServiceFlags:
 def get_many(key_list):
     """Acquire resource lock. Get the list of postconf values as specified.
     Return a key-value map"""
-    result = {}
     for key in key_list:
         validate_key(key)
     with postconf_mutex.lock_all():
-        for key in key_list:
+        return get_many_unsafe(key_list)
-            result[key] = get_unsafe(key)
+
-        return result
+
+def get_many_unsafe(key_iterator):
+    result = {}
+    for key in key_iterator:
+        result[key] = get_unsafe(key)
+    return result
 
 
 def set_many(kv_map):
@@ -45,8 +49,12 @@ def set_many(kv_map):
         validate_value(value)
 
     with postconf_mutex.lock_all():
-        for key, value in kv_map.items():
+        set_many_unsafe(kv_map)
-            set_unsafe(key, value)
+
+
+def set_many_unsafe(kv_map):
+    for key, value in kv_map.items():
+        set_unsafe(key, value)
 
 
 def set_master_cf_options(service_flags, options):