Add decorators for routing definitions

[JosXa]

So I've had this idea brewing in my head and was also approached by someone about it, therefore it's that time again where you need to file an Issue. The idea is the following:

Add an optional way of decorating a callback handler with its routing definitions. All our CommandHandlers, MessageHandlers etc. would get their corresponding decorators that can be used on top of callbacks, i.e. like this:

from mybot.dispatcher import command_handler, message_handler, callback_handler


@message_handler(Filters.group & Filters.reply)
def reply_in_group(bot, update):
    pass


@command_handler("start")
@command_handler("help")
def help(bot, update):
    # Multiple handlers for a callback
    update.message.reply_text("Help message")


@callback_handler(pattern='my-cb')
def button_callback(bot, update):
    pass

This was the tl;dr part, for some deeper examination, read on.

Current state

Now traditionally, we've had all our handlers+callback setup in a routing block which is, for most users anyway, located near the instantiation of updater and dispatcher. Some might have gone so far as to put all the dispatcher.add_handler calls inside its own file (e.g. routing.py), providing a convenient top-down/outside-in access to all the handlers. The workflow with this is something like need to change the name of a command or add some synonyms? No problem, know where to look, it's all right there...

What I usually do is to have logically related components that are actually reusable, but this only makes sense on larger scale and is not efficient for smaller bots.

In this philosophy, the routing happens outside of the handlers, in that they're decoupled from the place where they're used. I'm not sure if this was a concious design decision, or a construct that established over time... @jh0ker?
However, @Eldinnie made me aware that we are already breaking this convention by defining flow actions inside callbacks with the return values of ConversationHandlers, which is another reason I'm not holding back anymore with this enhancement proposal.

So far, so good. This philosophy works great, especially for larger projects, so I wouldn't wanna drop or change it.

With decorators

The second option allows for a setup where you can tell from a single glance at a callback signature how it is used and which bot functionalities are affected by this particular handler, increasing readability for you and discoverability for others reading your code, as well as code locality.

This also fixes the reusability of individual handlers in small bot environments. Say you have a command /developer (dev info) that you want to reuse in a couple of projects, then it will be cumbersome and inefficient to copy-paste the handler, and also search for its corresponding dispatcher.add_handler line. With a decorator, everything is in the same spot: Easy to give advice in the Telegram group or update our snippets with routing examples that don't feel out of place.

A point to consider is that is that by coupling the routing logic with a handler, you'd actually be decoupling the dispatcher object away from a "single point of use", forcing its distribution across all of your handler modules, making all these modules dependent on a dispatcher object. In the components modularization I described above, it makes sense to have a method register(dispatcher) within the module that takes a dispatcher as argument and preserves the traditional direction of dependencies. With routing, you need to actually import the dispatcher (which might, or might not be a good idea, again depending on project size).

I would like to offer both options to our users. I don't see a reason why not to, but please come forward if you see problems with this method other that the ones I mentioned, and let me know what you think about it overall.

I'm aware that @tsnoam, one of the core devs who will genuinely hate me for bringing up this idea, is categorically against decorators, but I know a lot of people that like and see value in them. It is common practice in many other bot and web frameworks, particularly the folks at pyTelegramBotAPI who use this method to their advantage, providing a lot of utility and readability.

We're also gonna have to think about border cases like the conversation handler, perhaps we can come up with a nice solution for it.

This would also be a good opportunity to rethink #859, because all the pass_* denotations are going to be cumbersome, make the line very long, and add no value in terms of readability since the associated part where they're actually required is just one line below in the method signature (and not miles away in the routing block). Perhaps a hybrid approach would be suitable, where only the "already inherently magical" decorators are capable of autowiring, but not the traditional handler classes...

[Lonami]

Regarding:

@message_handler(Filters.group & Filters.reply)

I feel like accepting *args (or even a tuple) would be a better option:

@message_handler(Filters.group, Filters.reply)
# or
@message_handler((Filters.group, Filters.reply))

Using & (or and, for what it's worth) seems to imply that you could also use | (or or), but you actually can't. Using a comma (, ) seems to make more sense.

Being able to use this syntax:

@command_handler("help")
def help(bot, update):
    ...

Seems just perfect and convenient. It has a Flask-like aesthethic:

@app.route('/hello')
def hello():
    return 'Hello world!'

And it could even infer the name of the command to use just based on the method name if left empty. Or even use the returning value as the reply! I vote yes for this proposal.

[JosXa]

I feel like accepting *args (or even a tuple) would be a better option

Right, I just made that up. It should be the same signature as the normal handler at first.

[jh0ker]

@Lonami Using & and | to combine filters is already established, and they are not the same thing.
Filters.text | Filters.photo accepts a message if it's text or photo, Filters.text & Filters.reply on the other hand accepts a message if it's text and a reply.

Edit: BTW we are using & and | because we can overwrite these operators, as opposed to and and or.

[Lonami]

Good point I didn't notice about the feature of it having to be this and something else. I was thinking about more types of updates. I also didn't know you couldn't override and and or.

[wagnerluis1982]

I think this issue can be solved without retrocompatibility issues using non obtrusive decorators, i.e. that only annotate the functions (or methods). This way, after annotated, it's only a matter of run a processor to make the real calls to the dispatcher.

[JosXa]

So you have a "global" collection of routing definitions that are added when the bot starts, sounds reasonable.
But I see a problem with the ordering of the handlers - see, ptb bots rely on correct order of add_handler calls.

[wagnerluis1982]

@JosXa it's no global if we have something like that:

from telegram import BotDecorator
from telegram.ext import Updater

deco = BotDecorator()

@deco.command("help")
def help(bot, update):
    ...

@deco.command("start")
def start(bot, update):
    ...

@deco.error()
def on_error(bot, update, exc):
    ...

updater = Updater("token")
updater.dispatcher.add_handlers(deco)

About the order, it's the order of the decorated.

[JosXa]

If we go for this, we might aswell look into doing something similar to flask blueprints.
Imo order should be customizable by easier means than managing order of imports and decorations, blueprints would partly allow that.
deco.set_position(N) or so

[Lonami]

What about something like @bot.register(Filters.Command('/start'), order=n)?

[Bibo-Joshi]

We discussed this issue in the dev chat and realized that most of us are not comfortable with it (anymore), including @JosXa as OP.
One major problem we see is that people will loose the oversight over the order in which handler are registered - which they already do without decorators.
Also, defining a register(dispatcher) method on module level that takes care of setting registering the handlers already is a viable middle way that allows you to set up the handlers in the same spot where you define the callbacks.

Therefore, I'll close this issue.