From ae2714c1f31f2a843e19dc44501784401181162c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 17 Aug 2021 12:23:14 +0200 Subject: [PATCH] Allow using several custom template directories (#10587) Allow using several directories in read_templates. --- changelog.d/10587.misc | 1 + synapse/config/_base.py | 41 +++++++++++-------- synapse/config/account_validity.py | 2 +- synapse/config/emailconfig.py | 8 ++-- synapse/config/sso.py | 2 +- synapse/module_api/__init__.py | 5 ++- tests/config/test_base.py | 64 ++++++++++++++++++++++++++++-- 7 files changed, 97 insertions(+), 26 deletions(-) create mode 100644 changelog.d/10587.misc diff --git a/changelog.d/10587.misc b/changelog.d/10587.misc new file mode 100644 index 0000000000..4c6167977c --- /dev/null +++ b/changelog.d/10587.misc @@ -0,0 +1 @@ +Allow multiple custom directories in `read_templates`. diff --git a/synapse/config/_base.py b/synapse/config/_base.py index d6ec618f8f..2cc242782a 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -237,13 +237,14 @@ class Config: def read_templates( self, filenames: List[str], - custom_template_directory: Optional[str] = None, + custom_template_directories: Optional[Iterable[str]] = None, ) -> List[jinja2.Template]: """Load a list of template files from disk using the given variables. This function will attempt to load the given templates from the default Synapse - template directory. If `custom_template_directory` is supplied, that directory - is tried first. + template directory. If `custom_template_directories` is supplied, any directory + in this list is tried (in the order they appear in the list) before trying + Synapse's default directory. Files read are treated as Jinja templates. The templates are not rendered yet and have autoescape enabled. @@ -251,8 +252,8 @@ class Config: Args: filenames: A list of template filenames to read. - custom_template_directory: A directory to try to look for the templates - before using the default Synapse template directory instead. + custom_template_directories: A list of directory to try to look for the + templates before using the default Synapse template directory instead. Raises: ConfigError: if the file's path is incorrect or otherwise cannot be read. @@ -260,20 +261,26 @@ class Config: Returns: A list of jinja2 templates. """ - search_directories = [self.default_template_dir] + search_directories = [] - # The loader will first look in the custom template directory (if specified) for the - # given filename. If it doesn't find it, it will use the default template dir instead - if custom_template_directory: - # Check that the given template directory exists - if not self.path_exists(custom_template_directory): - raise ConfigError( - "Configured template directory does not exist: %s" - % (custom_template_directory,) - ) + # The loader will first look in the custom template directories (if specified) + # for the given filename. If it doesn't find it, it will use the default + # template dir instead. + if custom_template_directories is not None: + for custom_template_directory in custom_template_directories: + # Check that the given template directory exists + if not self.path_exists(custom_template_directory): + raise ConfigError( + "Configured template directory does not exist: %s" + % (custom_template_directory,) + ) - # Search the custom template directory as well - search_directories.insert(0, custom_template_directory) + # Search the custom template directory as well + search_directories.append(custom_template_directory) + + # Append the default directory at the end of the list so Jinja can fallback on it + # if a template is missing from any custom directory. + search_directories.append(self.default_template_dir) # TODO: switch to synapse.util.templates.build_jinja_env loader = jinja2.FileSystemLoader(search_directories) diff --git a/synapse/config/account_validity.py b/synapse/config/account_validity.py index 6be4eafe55..9acce5996e 100644 --- a/synapse/config/account_validity.py +++ b/synapse/config/account_validity.py @@ -88,5 +88,5 @@ class AccountValidityConfig(Config): "account_previously_renewed.html", invalid_token_template_filename, ], - account_validity_template_dir, + (td for td in (account_validity_template_dir,) if td), ) diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 42526502f0..fc74b4a8b9 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -257,7 +257,9 @@ class EmailConfig(Config): registration_template_success_html, add_threepid_template_success_html, ], - template_dir, + ( + td for td in (template_dir,) if td + ), # Filter out template_dir if not provided ) # Render templates that do not contain any placeholders @@ -297,7 +299,7 @@ class EmailConfig(Config): self.email_notif_template_text, ) = self.read_templates( [notif_template_html, notif_template_text], - template_dir, + (td for td in (template_dir,) if td), ) self.email_notif_for_new_users = email_config.get( @@ -320,7 +322,7 @@ class EmailConfig(Config): self.account_validity_template_text, ) = self.read_templates( [expiry_template_html, expiry_template_text], - template_dir, + (td for td in (template_dir,) if td), ) subjects_config = email_config.get("subjects", {}) diff --git a/synapse/config/sso.py b/synapse/config/sso.py index d0f04cf8e6..4b590e0535 100644 --- a/synapse/config/sso.py +++ b/synapse/config/sso.py @@ -63,7 +63,7 @@ class SSOConfig(Config): "sso_auth_success.html", "sso_auth_bad_user.html", ], - self.sso_template_dir, + (td for td in (self.sso_template_dir,) if td), ) # These templates have no placeholders, so render them here diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 1cc13fc97b..82725853bc 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -677,7 +677,10 @@ class ModuleApi: A list containing the loaded templates, with the orders matching the one of the filenames parameter. """ - return self._hs.config.read_templates(filenames, custom_template_directory) + return self._hs.config.read_templates( + filenames, + (td for td in (custom_template_directory,) if td), + ) class PublicRoomListManager: diff --git a/tests/config/test_base.py b/tests/config/test_base.py index 84ae3b88ae..baa5313fb3 100644 --- a/tests/config/test_base.py +++ b/tests/config/test_base.py @@ -30,7 +30,7 @@ class BaseConfigTestCase(unittest.HomeserverTestCase): # contain template files with tempfile.TemporaryDirectory() as tmp_dir: # Attempt to load an HTML template from our custom template directory - template = self.hs.config.read_templates(["sso_error.html"], tmp_dir)[0] + template = self.hs.config.read_templates(["sso_error.html"], (tmp_dir,))[0] # If no errors, we should've gotten the default template instead @@ -60,7 +60,7 @@ class BaseConfigTestCase(unittest.HomeserverTestCase): # Attempt to load the template from our custom template directory template = ( - self.hs.config.read_templates([template_filename], tmp_dir) + self.hs.config.read_templates([template_filename], (tmp_dir,)) )[0] # Render the template @@ -74,8 +74,66 @@ class BaseConfigTestCase(unittest.HomeserverTestCase): "Template file did not contain our test string", ) + def test_multiple_custom_template_directories(self): + """Tests that directories are searched in the right order if multiple custom + template directories are provided. + """ + # Create two temporary directories on the filesystem. + tempdirs = [ + tempfile.TemporaryDirectory(), + tempfile.TemporaryDirectory(), + ] + + # Create one template in each directory, whose content is the index of the + # directory in the list. + template_filename = "my_template.html.j2" + for i in range(len(tempdirs)): + tempdir = tempdirs[i] + template_path = os.path.join(tempdir.name, template_filename) + + with open(template_path, "w") as fp: + fp.write(str(i)) + fp.flush() + + # Retrieve the template. + template = ( + self.hs.config.read_templates( + [template_filename], + (td.name for td in tempdirs), + ) + )[0] + + # Test that we got the template we dropped in the first directory in the list. + self.assertEqual(template.render(), "0") + + # Add another template, this one only in the second directory in the list, so we + # can test that the second directory is still searched into when no matching file + # could be found in the first one. + other_template_name = "my_other_template.html.j2" + other_template_path = os.path.join(tempdirs[1].name, other_template_name) + + with open(other_template_path, "w") as fp: + fp.write("hello world") + fp.flush() + + # Retrieve the template. + template = ( + self.hs.config.read_templates( + [other_template_name], + (td.name for td in tempdirs), + ) + )[0] + + # Test that the file has the expected content. + self.assertEqual(template.render(), "hello world") + + # Cleanup the temporary directories manually since we're not using a context + # manager. + for td in tempdirs: + td.cleanup() + def test_loading_template_from_nonexistent_custom_directory(self): with self.assertRaises(ConfigError): self.hs.config.read_templates( - ["some_filename.html"], "a_nonexistent_directory" + ["some_filename.html"], ("a_nonexistent_directory",) )