diff --git a/CHANGES.rst b/CHANGES.rst index f5a484590..cd2589580 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -46,6 +46,7 @@ fixes: - docs: add command parameter section and minor doc edits (#1740) - chore: bump actions/upload-artifact version (#1762) - docs: add positional argument with default example (#1763) +- feat: unique template names (#1764) v6.2.0 (2024-01-01) diff --git a/docs/user_guide/plugin_development/messaging.rst b/docs/user_guide/plugin_development/messaging.rst index 8bac92fa1..0b4733119 100644 --- a/docs/user_guide/plugin_development/messaging.rst +++ b/docs/user_guide/plugin_development/messaging.rst @@ -91,9 +91,32 @@ template (`{{name}}` in the above template example): """Say hello to someone""" return {'name': args} -It's also possible to use templates when using `self.send()`, but in -this case you will have to do the template rendering step yourself, -like so: +.. note:: + Templates are namespaced by plugin name to avoid collisions between plugins. + The namespace is the name of your plugin as defined in the `[Core]` section of + your `.plug` file. If you have multiple plugins with the same template name + (e.g. `issue.md`), Errbot will prioritize the one belonging to the plugin + currently executing the command. + +It's also possible to use templates when using `self.send_templated()`, which +is the preferred way to send templated messages from pollers or webhooks. It +simplifies your code by handling both the template rendering and the plugin +namespacing automatically in a single call: + +.. code-block:: python + + from errbot import BotPlugin, botcmd + + class Hello(BotPlugin): + @botcmd + def hello(self, msg, args): + """Say hello to someone""" + self.send_templated(msg.frm, 'hello', {'name': args}) + +If you really need to do the template rendering step yourself, you can use +:func:`~errbot.templating.tenv`, but be aware that you might need to prefix +the template name with your plugin name to avoid collisions if you are not +using `self.send_templated()`: .. code-block:: python @@ -104,7 +127,8 @@ like so: @botcmd(template="hello") def hello(self, msg, args): """Say hello to someone""" - response = tenv().get_template('hello.md').render(name=args) + # Explicitly using the plugin namespace to avoid collisions + response = tenv().get_template('Hello/hello.md').render(name=args) self.send(msg.frm, response) diff --git a/errbot/botplugin.py b/errbot/botplugin.py index d7a709a43..b17893324 100644 --- a/errbot/botplugin.py +++ b/errbot/botplugin.py @@ -701,6 +701,7 @@ def send_templated( template_parameters=template_parameters, in_reply_to=in_reply_to, groupchat_nick_reply=groupchat_nick_reply, + plugin_name=self.name, ) def build_identifier(self, txtrep: str) -> Identifier: diff --git a/errbot/core.py b/errbot/core.py index f693e8f14..fa3dcceb1 100644 --- a/errbot/core.py +++ b/errbot/core.py @@ -192,6 +192,7 @@ def send_templated( template_parameters, in_reply_to: Optional[Message] = None, groupchat_nick_reply: bool = False, + plugin_name: str = None, ) -> Callable: """Sends a simple message to the specified user using a template. @@ -203,8 +204,12 @@ def send_templated( the original message the bot is answering from :param groupchat_nick_reply: authorized the prefixing with the nick form the user + :param plugin_name: + the name of the plugin that is calling this method, to use it as a template namespace. """ - text = self.process_template(template_name, template_parameters) + text = self.process_template( + template_name, template_parameters, plugin_name=plugin_name + ) return self.send(identifier, text, in_reply_to, groupchat_nick_reply) def split_and_send_message(self, msg: Message) -> None: @@ -504,14 +509,32 @@ def _process_command(self, msg, cmd, args, match): ) @staticmethod - def process_template(template_name, template_parameters): + def process_template(template_name, template_parameters, plugin_name: str = None): + """ + Processes a template with the given parameters. + + :param template_name: the name of the template to use. + :param template_parameters: the parameters to pass to the template. + :param plugin_name: the name of the plugin to use as a template namespace. + """ # integrated templating # The template needs to be set and the answer from the user command needs to be a mapping # If not just convert the answer to string. if template_name and isinstance(template_parameters, Mapping): - return ( - tenv().get_template(template_name + ".md").render(**template_parameters) - ) + t_name = template_name + ".md" + if plugin_name: + try: + return ( + tenv() + .get_template(f"{plugin_name}/{t_name}") + .render(**template_parameters) + ) + except Exception: + log.debug( + f"Template {t_name} not found in plugin {plugin_name} namespace, falling back to global search." + ) + + return tenv().get_template(t_name).render(**template_parameters) # Reply should be all text at this point (See https://github.com/errbotio/errbot/issues/96) return str(template_parameters) @@ -554,13 +577,16 @@ def _execute_and_send(self, cmd, args, match, msg, template_name=None): ) return + plugin_name = getattr(getattr(method, "__self__", None), "name", None) if inspect.isgeneratorfunction(method): replies = method(msg, match) if match else method(msg, args) for reply in replies: if reply: self.send_simple_reply( msg, - self.process_template(template_name, reply), + self.process_template( + template_name, reply, plugin_name=plugin_name + ), private, threaded, ) @@ -569,7 +595,9 @@ def _execute_and_send(self, cmd, args, match, msg, template_name=None): if reply: self.send_simple_reply( msg, - self.process_template(template_name, reply), + self.process_template( + template_name, reply, plugin_name=plugin_name + ), private, threaded, ) @@ -580,7 +608,9 @@ def _execute_and_send(self, cmd, args, match, msg, template_name=None): except CommandError as command_error: reason = command_error.reason if command_error.template: - reason = self.process_template(command_error.template, reason) + reason = self.process_template( + command_error.template, reason, plugin_name=plugin_name + ) self.send_simple_reply(msg, reason, private, threaded) except Exception as e: diff --git a/errbot/templating.py b/errbot/templating.py index 35ecc4ae7..b2f988714 100644 --- a/errbot/templating.py +++ b/errbot/templating.py @@ -1,7 +1,7 @@ import logging from pathlib import Path -from jinja2 import Environment, FileSystemLoader +from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PrefixLoader from errbot.plugin_info import PluginInfo @@ -14,12 +14,25 @@ def make_templates_path(root: Path) -> Path: system_templates_path = str(make_templates_path(Path(__file__).parent)) template_path = [system_templates_path] -env = Environment( - loader=FileSystemLoader(template_path), - trim_blocks=True, - keep_trailing_newline=False, - autoescape=True, -) +plugin_templates = {} # plugin_name -> FileSystemLoader + + +def _recreate_env(): + global env + loaders = [] + if plugin_templates: + loaders.append(PrefixLoader(plugin_templates)) + loaders.append(FileSystemLoader(template_path)) + + env = Environment( + loader=ChoiceLoader(loaders), + trim_blocks=True, + keep_trailing_newline=False, + autoescape=True, + ) + + +_recreate_env() def tenv() -> Environment: @@ -27,16 +40,16 @@ def tenv() -> Environment: def add_plugin_templates_path(plugin_info: PluginInfo) -> None: - global env tmpl_path = make_templates_path(plugin_info.location.parent) if tmpl_path.exists(): log.debug( "Templates directory found for %s plugin [%s]", plugin_info.name, tmpl_path ) template_path.append(str(tmpl_path)) # for webhooks + plugin_templates[plugin_info.name] = FileSystemLoader(str(tmpl_path)) # Ditch and recreate a new templating environment - env = Environment(loader=FileSystemLoader(template_path), autoescape=True) + _recreate_env() return log.debug( "No templates directory found for %s plugin in [%s]", @@ -46,9 +59,16 @@ def add_plugin_templates_path(plugin_info: PluginInfo) -> None: def remove_plugin_templates_path(plugin_info: PluginInfo) -> None: - global env tmpl_path = str(make_templates_path(plugin_info.location.parent)) + changed = False if tmpl_path in template_path: template_path.remove(tmpl_path) + changed = True + + if plugin_info.name in plugin_templates: + del plugin_templates[plugin_info.name] + changed = True + + if changed: # Ditch and recreate a new templating environment - env = Environment(loader=FileSystemLoader(template_path), autoescape=True) + _recreate_env() diff --git a/tests/template_plugin/collision_a/collision_a.plug b/tests/template_plugin/collision_a/collision_a.plug new file mode 100644 index 000000000..6c0784b58 --- /dev/null +++ b/tests/template_plugin/collision_a/collision_a.plug @@ -0,0 +1,6 @@ +[Core] +Name = CollisionA +Module = collision_a + +[Python] +Version = 3 diff --git a/tests/template_plugin/collision_a/collision_a.py b/tests/template_plugin/collision_a/collision_a.py new file mode 100644 index 000000000..a64be087d --- /dev/null +++ b/tests/template_plugin/collision_a/collision_a.py @@ -0,0 +1,10 @@ +from errbot import BotPlugin, botcmd + +class CollisionA(BotPlugin): + @botcmd(template='collision') + def test_a(self, msg, args): + return {'name': 'PluginA'} + + @botcmd + def test_manual(self, msg, args): + self.send_templated(msg.frm, 'collision', {'name': 'PluginA'}) diff --git a/tests/template_plugin/collision_a/templates/collision.md b/tests/template_plugin/collision_a/templates/collision.md new file mode 100644 index 000000000..f8fdf9690 --- /dev/null +++ b/tests/template_plugin/collision_a/templates/collision.md @@ -0,0 +1 @@ +Template from {{ name }} diff --git a/tests/template_plugin/collision_b/collision_b.plug b/tests/template_plugin/collision_b/collision_b.plug new file mode 100644 index 000000000..f94fd5bab --- /dev/null +++ b/tests/template_plugin/collision_b/collision_b.plug @@ -0,0 +1,6 @@ +[Core] +Name = CollisionB +Module = collision_b + +[Python] +Version = 3 diff --git a/tests/template_plugin/collision_b/collision_b.py b/tests/template_plugin/collision_b/collision_b.py new file mode 100644 index 000000000..ef8ecb7fb --- /dev/null +++ b/tests/template_plugin/collision_b/collision_b.py @@ -0,0 +1,6 @@ +from errbot import BotPlugin, botcmd + +class CollisionB(BotPlugin): + @botcmd(template='collision') + def test_b(self, msg, args): + return {'name': 'PluginB'} diff --git a/tests/template_plugin/collision_b/templates/collision.md b/tests/template_plugin/collision_b/templates/collision.md new file mode 100644 index 000000000..c8971de90 --- /dev/null +++ b/tests/template_plugin/collision_b/templates/collision.md @@ -0,0 +1 @@ +Template from {{ name }} (B) diff --git a/tests/templates_test.py b/tests/templates_test.py index ed2d6b83b..5382b120d 100644 --- a/tests/templates_test.py +++ b/tests/templates_test.py @@ -25,3 +25,20 @@ def test_templates_5(testbot): assert "the following arguments are required: my_var" in testbot.exec_command( "!test template4" ) + + +def test_template_collision(testbot): + # Test CollisionA (namespaced) + testbot.push_message("!test_a") + response = testbot.pop_message() + assert "Template from PluginA" in response + + # Test CollisionB (namespaced) + testbot.push_message("!test_b") + response = testbot.pop_message() + assert "Template from PluginB (B)" in response + + # Test manual send_templated (respects namespace) + testbot.push_message("!test_manual") + response = testbot.pop_message() + assert "Template from PluginA" in response