5 files changed, 85 insertions, 37 deletions

diff --git a/src/mailman/app/templates.py b/src/mailman/app/templates.py
index 7d34e9ffc..1d1203ad7 100644
--- a/src/mailman/app/templates.py
+++ b/src/mailman/app/templates.py
@@ -39,9 +39,10 @@ from zope.interface import implementer
 class MailmanHandler(BaseHandler):
     # Handle internal mailman: URLs.
     def mailman_open(self, req):
+        list_manager = getUtility(IListManager)
         # Parse urls of the form:
         #
-        # mailman:///<fqdn_listname>/<language>/<template_name>
+        # mailman:///<fqdn_listname|list_id>/<language>/<template_name>
         #
         # where only the template name is required.
         mlist = code = template = None
@@ -58,19 +59,25 @@ class MailmanHandler(BaseHandler):
             template = parts[0]
         elif len(parts) == 2:
             part0, template = parts
-            # Is part0 a language code or a mailing list?  It better be one or
-            # the other, and there's no possibility of namespace collisions
-            # because language codes don't contain @ and mailing list names
-            # MUST contain @.
+            # Is part0 a language code or a mailing list?  This is rather
+            # tricky because if it's a mailing list, it could be a list-id and
+            # that will contain dots, as could the language code.
             language = getUtility(ILanguageManager).get(part0)
-            mlist = getUtility(IListManager).get(part0)
-            if language is None and mlist is None:
-                raise URLError('Bad language or list name')
-            elif mlist is None:
+            if language is None:
+                # part0 must be a fqdn-listname or list-id.
+                mlist = (list_manager.get(part0)
+                         if '@' in part0 else
+                         list_manager.get_by_list_id(part0))
+                if mlist is None:
+                    raise URLError('Bad language or list name')
+            else:
                 code = language.code
         elif len(parts) == 3:
-            fqdn_listname, code, template = parts
-            mlist = getUtility(IListManager).get(fqdn_listname)
+            part0, code, template = parts
+            # part0 could be an fqdn-listname or a list-id.
+            mlist = (getUtility(IListManager).get(part0)
+                     if '@' in part0 else
+                     getUtility(IListManager).get_by_list_id(part0))
             if mlist is None:
                 raise URLError('Missing list')
             language = getUtility(ILanguageManager).get(code)
diff --git a/src/mailman/handlers/decorate.py b/src/mailman/handlers/decorate.py
index cd9f189aa..8493a0b08 100644
--- a/src/mailman/handlers/decorate.py
+++ b/src/mailman/handlers/decorate.py
@@ -217,8 +217,9 @@ def decorate(mlist, uri, extradict=None):
     # Get the decorator template.
     loader = getUtility(ITemplateLoader)
     template_uri = expand(uri, dict(
-        listname=mlist.fqdn_listname,
         language=mlist.preferred_language.code,
+        list_id=mlist.list_id,
+        listname=mlist.fqdn_listname,
         ))
     template = loader.get(template_uri)
     return decorate_template(mlist, template, extradict)
diff --git a/src/mailman/handlers/tests/test_decorate.py b/src/mailman/handlers/tests/test_decorate.py
index 9707223c5..84adceb9a 100644
--- a/src/mailman/handlers/tests/test_decorate.py
+++ b/src/mailman/handlers/tests/test_decorate.py
@@ -23,8 +23,6 @@ __all__ = [
 
 
 import os
-import shutil
-import tempfile
 import unittest
 
 from mailman.app.lifecycle import create_list
@@ -33,6 +31,7 @@ from mailman.handlers import decorate
 from mailman.interfaces.archiver import IArchiver
 from mailman.testing.helpers import specialized_message_from_string as mfs
 from mailman.testing.layers import ConfigLayer
+from tempfile import TemporaryDirectory
 from zope.interface import implementer
 
 
@@ -56,19 +55,18 @@ class TestDecorate(unittest.TestCase):
     layer = ConfigLayer
 
     def setUp(self):
-        self._mlist = create_list('test@example.com')
+        self._mlist = create_list('ant@example.com')
         self._msg = mfs("""\
-To: test@example.com
+To: ant@example.com
 From: aperson@example.com
-Message-ID: <somerandomid.example.com>
+Message-ID: <alpha>
 Content-Type: text/plain;
 
 This is a test message.
 """)
-        template_dir = tempfile.mkdtemp()
-        self.addCleanup(shutil.rmtree, template_dir)
-        site_dir = os.path.join(template_dir, 'site', 'en')
-        os.makedirs(site_dir)
+        temporary_dir = TemporaryDirectory()
+        self.addCleanup(temporary_dir.cleanup)
+        template_dir = temporary_dir.name
         config.push('archiver', """\
         [paths.testing]
         template_dir: {}
@@ -77,13 +75,44 @@ This is a test message.
         enable: yes
         """.format(template_dir))
         self.addCleanup(config.pop, 'archiver')
-        self.footer_path = os.path.join(site_dir, 'myfooter.txt')
 
     def test_decorate_footer_with_archive_url(self):
-        with open(self.footer_path, 'w', encoding='utf-8') as fp:
+        site_dir = os.path.join(config.TEMPLATE_DIR, 'site', 'en')
+        os.makedirs(site_dir)
+        footer_path = os.path.join(site_dir, 'myfooter.txt')
+        with open(footer_path, 'w', encoding='utf-8') as fp:
             print('${testarchiver_url}', file=fp)
         self._mlist.footer_uri = 'mailman:///myfooter.txt'
         self._mlist.preferred_language = 'en'
         decorate.process(self._mlist, self._msg, {})
         self.assertIn('http://example.com/link_to_message',
                       self._msg.as_string())
+
+    def test_list_id_allowed_in_template_uri(self):
+        # Issue #196 - allow the list_id in the template uri expansion.
+        list_dir = os.path.join(
+            config.TEMPLATE_DIR, 'lists', 'ant.example.com', 'en')
+        os.makedirs(list_dir)
+        footer_path = os.path.join(list_dir, 'myfooter.txt')
+        with open(footer_path, 'w', encoding='utf-8') as fp:
+            print('${testarchiver_url}', file=fp)
+        self._mlist.footer_uri = 'mailman:///${list_id}/myfooter.txt'
+        self._mlist.preferred_language = 'en'
+        decorate.process(self._mlist, self._msg, {})
+        self.assertIn('http://example.com/link_to_message',
+                      self._msg.as_string())
+
+    def test_list_id_and_language_code_allowed_in_template_uri(self):
+        # Issue #196 - allow the list_id in the template uri expansion.
+        list_dir = os.path.join(
+            config.TEMPLATE_DIR, 'lists', 'ant.example.com', 'it')
+        os.makedirs(list_dir)
+        footer_path = os.path.join(list_dir, 'myfooter.txt')
+        with open(footer_path, 'w', encoding='utf-8') as fp:
+            print('${testarchiver_url}', file=fp)
+        self._mlist.footer_uri = (
+            'mailman:///${list_id}/${language}/myfooter.txt')
+        self._mlist.preferred_language = 'it'
+        decorate.process(self._mlist, self._msg, {})
+        self.assertIn('http://example.com/link_to_message',
+                      self._msg.as_string())
diff --git a/src/mailman/utilities/i18n.py b/src/mailman/utilities/i18n.py
index bce1026f6..069bd8f52 100644
--- a/src/mailman/utilities/i18n.py
+++ b/src/mailman/utilities/i18n.py
@@ -27,7 +27,6 @@ __all__ = [
 
 import os
 import sys
-import errno
 
 from itertools import product
 from mailman.config import config
@@ -61,7 +60,8 @@ def search(template_file, mlist=None, language=None):
     The <language> path component is variable, and described below.
 
     * The list-specific language directory
-      $template_dir/lists/<mlist.fqdn_listname>/<language>
+      $template_dir/lists/<mlist.list_id>/<language>
+      $template_dir/lists/<mlist.fqdn_listname>/<language> (deprecated)
 
     * The domain-specific language directory
       $template_dir/domains/<mlist.mail_host>/<language>
@@ -78,23 +78,27 @@ def search(template_file, mlist=None, language=None):
 
     Languages are iterated after each of the four locations are searched.  So
     for example, when searching for the 'foo.txt' template, where the server's
-    default language is 'fr', the mailing list's (test@example.com) language
+    default language is 'fr', the mailing list's (test.example.com) language
     is 'de' and the `language` parameter is 'it', these locations are searched
     in order:
 
-    * $template_dir/lists/test@example.com/it/foo.txt
+    * $template_dir/lists/test.example.com/it/foo.txt
+    * $template_dir/lists/test@example.com/it/foo.txt (deprecated)
     * $template_dir/domains/example.com/it/foo.txt
     * $template_dir/site/it/foo.txt
 
-    * $template_dir/lists/test@example.com/de/foo.txt
+    * $template_dir/lists/test.example.com/de/foo.txt
+    * $template_dir/lists/test@example.com/de/foo.txt (deprecated)
     * $template_dir/domains/example.com/de/foo.txt
     * $template_dir/site/de/foo.txt
 
-    * $template_dir/lists/test@example.com/fr/foo.txt
+    * $template_dir/lists/test.example.com/fr/foo.txt
+    * $template_dir/lists/test@example.com/fr/foo.txt (deprecated)
     * $template_dir/domains/example.com/fr/foo.txt
     * $template_dir/site/fr/foo.txt
 
-    * $template_dir/lists/test@example.com/en/foo.txt
+    * $template_dir/lists/test.example.com/en/foo.txt
+    * $template_dir/lists/test@example.com/en/foo.txt (deprecated)
     * $template_dir/domains/example.com/en/foo.txt
     * $template_dir/site/en/foo.txt
 
@@ -113,10 +117,13 @@ def search(template_file, mlist=None, language=None):
     # The non-language qualified $template_dir paths in search order.
     paths = [os.path.join(config.TEMPLATE_DIR, 'site')]
     if mlist is not None:
+        # Don't forget these are in REVERSE search order!
         paths.append(os.path.join(
             config.TEMPLATE_DIR, 'domains', mlist.mail_host))
         paths.append(os.path.join(
             config.TEMPLATE_DIR, 'lists', mlist.fqdn_listname))
+        paths.append(os.path.join(
+            config.TEMPLATE_DIR, 'lists', mlist.list_id))
     paths.reverse()
     for language, path in product(languages, paths):
         yield os.path.join(path, language, template_file)
@@ -151,15 +158,12 @@ def find(template_file, mlist=None, language=None, _trace=False):
             if _trace:
                 print('@@@', path, end='', file=sys.stderr)
             fp = open(path, 'r', encoding='utf-8')
-        except IOError as error:
-            if error.errno == errno.ENOENT:
-                if _trace:
-                    print('MISSING', file=sys.stderr)
-            else:
-                raise
+        except FileNotFoundError:
+            if _trace:
+                print(' MISSING', file=sys.stderr)
         else:
             if _trace:
-                print('FOUND:', path, file=sys.stderr)
+                print(' FOUND:', path, file=sys.stderr)
             return path, fp
     raise TemplateNotFoundError(template_file)
 
diff --git a/src/mailman/utilities/tests/test_templates.py b/src/mailman/utilities/tests/test_templates.py
index 8697699ab..f4b581b90 100644
--- a/src/mailman/utilities/tests/test_templates.py
+++ b/src/mailman/utilities/tests/test_templates.py
@@ -88,18 +88,22 @@ class TestSearchOrder(unittest.TestCase):
         def nexteq(path):
             self.assertEqual(next(search_order), path)
         # 1: Use the given language argument
+        nexteq('/v/templates/lists/l.example.com/it/foo.txt')
         nexteq('/v/templates/lists/l@example.com/it/foo.txt')
         nexteq('/v/templates/domains/example.com/it/foo.txt')
         nexteq('/v/templates/site/it/foo.txt')
         # 2: Use mlist.preferred_language
+        nexteq('/v/templates/lists/l.example.com/de/foo.txt')
         nexteq('/v/templates/lists/l@example.com/de/foo.txt')
         nexteq('/v/templates/domains/example.com/de/foo.txt')
         nexteq('/v/templates/site/de/foo.txt')
         # 3: Use the site's default language
+        nexteq('/v/templates/lists/l.example.com/fr/foo.txt')
         nexteq('/v/templates/lists/l@example.com/fr/foo.txt')
         nexteq('/v/templates/domains/example.com/fr/foo.txt')
         nexteq('/v/templates/site/fr/foo.txt')
         # 4: English
+        nexteq('/v/templates/lists/l.example.com/en/foo.txt')
         nexteq('/v/templates/lists/l@example.com/en/foo.txt')
         nexteq('/v/templates/domains/example.com/en/foo.txt')
         nexteq('/v/templates/site/en/foo.txt')
@@ -114,14 +118,17 @@ class TestSearchOrder(unittest.TestCase):
         def nexteq(path):
             self.assertEqual(next(search_order), path)
         # 1: Use mlist.preferred_language
+        nexteq('/v/templates/lists/l.example.com/de/foo.txt')
         nexteq('/v/templates/lists/l@example.com/de/foo.txt')
         nexteq('/v/templates/domains/example.com/de/foo.txt')
         nexteq('/v/templates/site/de/foo.txt')
         # 2: Use the site's default language
+        nexteq('/v/templates/lists/l.example.com/fr/foo.txt')
         nexteq('/v/templates/lists/l@example.com/fr/foo.txt')
         nexteq('/v/templates/domains/example.com/fr/foo.txt')
         nexteq('/v/templates/site/fr/foo.txt')
         # 3: English
+        nexteq('/v/templates/lists/l.example.com/en/foo.txt')
         nexteq('/v/templates/lists/l@example.com/en/foo.txt')
         nexteq('/v/templates/domains/example.com/en/foo.txt')
         nexteq('/v/templates/site/en/foo.txt')