Added server command post hooks

[Ayuto]

This PR adds the possibility to register server command post hooks by adding a new parameter to <ServerCommandDispatcher>.add_callback/remove_callback(). I didn't update the ServerCommand decorator class, because it doesn't seem to be that easy. So, if you want to register a server command post hook, you need to use the low-level functions:

from commands.server import get_server_command
from memory.hooks import HookType

echo = get_server_command('echo')

def pre_echo(command):
    print('pre_echo', tuple(command))
    
def post_echo(command):
    print('post_echo', tuple(command))
    
echo.add_callback(pre_echo, HookType.PRE)
echo.add_callback(post_echo, HookType.POST)

def unload():
    echo.remove_callback(pre_echo, HookType.PRE)
    echo.remove_callback(post_echo, HookType.POST)

Output:

echo Hello World!
pre_echo ('echo', 'Hello', 'World!')
Hello World!
post_echo ('echo', 'Hello', 'World!')

[jordanbriere]

Nice! Here are few points:

• I believe we should raise if someone attempt to register the same callback twice rather than ignoring it and stay silent. For example:

  echo.add_callback(pre_echo, HookType.PRE)
  echo.add_callback(pre_echo, HookType.PRE)

  This should raise on the second call, otherwise the code indicates the callback should be registered
  and called twice while an exception is explicit that they are doing something wrong.
• The calling is currently inconsistent. For example:

  from commands import CommandReturn
  from commands.server import get_server_command
  from memory.hooks import HookType
  
  echo = get_server_command('echo')
  
  def pre_echo_continue(command):
     print('pre_echo_continue', tuple(command))
  
  def pre_echo_block(command):
     print('pre_echo_block', tuple(command))
     return CommandReturn.BLOCK
  
  def pre_echo_after_block(command):
     print('pre_echo_after_block', tuple(command))
  
  def post_echo(command):
     print('post_echo', tuple(command))
  
  echo.add_callback(pre_echo_continue, HookType.PRE)
  echo.add_callback(pre_echo_block, HookType.PRE)
  echo.add_callback(pre_echo_after_block, HookType.PRE)
  echo.add_callback(post_echo, HookType.POST)
  
  def unload():
     echo.remove_callback(pre_echo_continue, HookType.PRE)
     echo.remove_callback(pre_echo_block, HookType.PRE)
     echo.remove_callback(pre_echo_after_block, HookType.PRE)
     echo.remove_callback(post_echo, HookType.POST)

  Result:

  echo hey
  pre_echo_continue ('echo', 'hey')
  pre_echo_block ('echo', 'hey')
  post_echo ('echo', 'hey')
  

  Since the behavior of the pre callbacks list is to stop whenever one of them block the command, the post callbacks should also not be processed in such case, for consistency. Another possibility could be to pass the blocked status to the callbacks so they can determine themselves if they want to process or exit the call.

[jordanbriere]

I didn't update the ServerCommand decorator class, because it doesn't seem to be that easy.

I just made a quick test and it was not so hard to implement. The following seems to work fine:

# ../commands/server.py

"""Provides say command functionality."""

# =============================================================================
# >> FORWARD IMPORTS
# =============================================================================
# Source.Python Imports
#   Commands
from _commands._server import ServerCommandDispatcher
from _commands._server import ServerCommandGenerator
from _commands._server import get_server_command
from commands.command import _BaseCommand
from commands.manager import _BaseCommandManager
#   Memory
from memory.hooks import HookType


# =============================================================================
# >> ALL DECLARATION
# =============================================================================
__all__ = ('ServerCommand',
           'ServerCommandDispatcher',
           'ServerCommandGenerator',
           '_ServerCommandManager',
           'get_server_command',
           'server_command_manager',
           )


# =============================================================================
# >> CLASSES
# =============================================================================
class _ServerCommandManager(_BaseCommandManager):
    """Manager class used to register server commands."""

    # Store the base functions
    _get_command = staticmethod(get_server_command)
    
    def register_commands(
            self, names, callback, hook_type=HookType.PRE, *args, **kwargs):
        """Register the given commands to the given callback.

        :param str/list/tuple names:
            Command names that should be registered.
        :param callable callback:
            A callback that should get called when one of the commands were
            issued.
        :param HookType hook_type:
            The hook type of the registered callback.
        :param args:
            Additional arguments that get passed to the proxy (if existant)
            and to :meth:`_register_command`.
        :param kwargs:
            Additional keywords that get passed to the proxy (if existant)
            and to :meth:`_register_command`.
        """
        names = self._prepare_command_names(names)

        if self._callback_proxy is None:
            for name in names:
                self._register_command(name, callback, hook_type, *args, **kwargs)
        else:
            proxy = self._callback_proxy(callback, *args, **kwargs)

            for name in names:
                self._callback_proxies[name].append(proxy)
                self._register_command(name, proxy, hook_type, *args, **kwargs)

    def _register_command(
            self, name, callback, hook_type, *args, **kwargs):
        """Register a command.

        :param str name:
            A single command name that should be registred.
        :param callable callback:
            The callback that should get called when the command has been issued.
        :param HookType hook_type:
            The hook type of the registered callback.
        :param args:
            Additional arguments passed from :meth:`register_commands`.
        :param kwargs:
            Additional keywords passed from :meth:`register_commands`.
        """
        self._get_command(name, *args).add_callback(callback, hook_type)

    def unregister_commands(self, names, callback, hook_type=HookType.PRE):
        """Unregister the given commands from the given callback.

        :param str/list/tuple names:
            Command names to unregister.
        :param callable callback:
            The callback that was registered for the commands.
        :param HookType hook_type:
            The hook type of the registered callback.
        """
        names = self._prepare_command_names(names)

        if self._callback_proxy is None:
            for name in names:
                self._unregister_command(name, callback, hook_type)
        else:
            for name in names:
                proxy = self._get_command_proxy(name, callback)
                self._callback_proxies[name].remove(proxy)

                if not self._callback_proxies[name]:
                    del self._callback_proxies[name]

                self._unregister_command(name, proxy, hook_type)

    def _unregister_command(self, name, callback, hook_type):
        """Unregister a command.

        :param str name:
            Name of the command to unregister.
        :param callable callback:
            The callback that is assigned to the command.
        :param HookType hook_type:
            The hook type of the registered callback.
        """
        self._get_command(name).remove_callback(callback, hook_type)

# The singleton object of the :class:`_ServerCommandManager` class
server_command_manager = _ServerCommandManager()


class _BaseServerCommand(_BaseCommand):
    def __init__(self, names, hook_type=HookType.PRE, *args, **kwargs):
        super().__init__(names, *args, **kwargs)
        self.hook_type = hook_type

    def __call__(self, callback):
        self.callback = callback
        self._manager_class.register_commands(
            self.names, self.callback, self.hook_type,
                *self.args, **self.kwargs)
        return self.callback

    def _unload_instance(self):
        self._manager_class.unregister_commands(
            self.names, self.callback, self.hook_type)


class ServerCommand(_BaseServerCommand):
    """Decorator class used to register a server command."""

    # Store the class used to (un)register server commands
    _manager_class = server_command_manager

Would requires more testing, but the following test looks promising:

from commands.server import ServerCommand
from memory.hooks import HookType

@ServerCommand('echo', HookType.PRE)
def pre_echo(command):
    print('pre_echo', tuple(command))

@ServerCommand('echo', HookType.POST)
def post_echo(command):
    print('post_echo', tuple(command))

Result:

echo hey
pre_echo ('echo', 'hey')
hey
post_echo ('echo', 'hey')

[Ayuto]

I believe we should raise if someone attempt to register the same callback twice rather than ignoring it and stay silent.

It's working like that since the beginning. Though, I fully agree with that. I just didn't want to change existing behaviour.

Since the behavior of the pre callbacks list is to stop whenever one of them block the command, the post callbacks should also not be processed in such case, for consistency.

Our function hooks work the same way. Though, they also call every pre-hook callback even if a previous callback wants to block the original function. So, we currently have a mixture of both behaviours.

I like your proposal:

Another possibility could be to pass the blocked status to the callbacks so they can determine themselves if they want to process or exit the call.

However, I really don't want to change existing behaviour or add a third behaviour.

I just made a quick test and it was not so hard to implement.

I should have added: without reimplementing the whole base class(es). :D

[jordanbriere]

It's working like that since the beginning. Though, I fully agree with that. I just didn't want to change existing behaviour.

Yes, I just never noticed until now. I think this can safely be changed without breaking any existing codes.

Our function hooks work the same way. Though, they also call every pre-hook callback even if a previous callback wants to block the original function. So, we currently have a mixture of both behaviours.

I tend to have a mixed opinion here. On one side I agree, and on the other I believe they are totally different mechanisms. The memory hooks are a two ways process while the commands are one way. Once the commands are blocked, nothing else happens while the memory hooks still have to return to the caller so giving the chance to all callbacks to voice their return value make senses, but executing post callbacks for a command that never happened seems broken to me. I mean, post what? Nothing happened... if that makes sense. 😛

I like your proposal:

Another possibility could be to pass the blocked status to the callbacks so they can determine themselves if they want to process or exit the call.

However, I really don't want to change existing behaviour or add a third behaviour.

I believe this is the only approach that makes it correct to call all callbacks regardless of the blocking state. As long as they are aware of it, it is up to them. We don't necessarily need to pass it as a second argument to the callbacks breaking all existing plugins. We really could just add a command_return property to the CCommand exports that simply bind the state to the Python object and return CONTINUE by default. Then from a callback we could simply do:

@ServerCommand('echo')
def on_echo(command):
    if command.command_return == CommandReturn.BLOCK:
        return # Command was blocked, nothing to do here.
    ...

Of course, with such implementation all pre and post callbacks should be called regardless of the command being blocked by a previous callback. At least now they have the choice and are aware of what will/or happen/ed.

I should have added: without reimplementing the whole base class(es). :D

Fair enough. 😆

[jordanbriere]

Good idea to use CListenerManager! 👍