Class-based namespaces

[bob1de]

Hi,

I've implemented namespace-based grouping of event handlers into classes as a little enhancement to the Server.on(...) method which is currently the only available way to register event handlers. As this can become muddled rather quickly I wrote this extension. It also cares about setting default namespaces when calling emit, disconnect etc. on such a Namespace object.

Tests and docs are updated as well.

Hope it gets merged and will help some people writing their Socket.IO apps more easily.

[miguelgrinberg]

This is an interesting idea. I'll play with this patch a bit before merging it, I think I want to make a minor change to have the namespace object is created, but overall I like the idea. Thanks!

[bob1de]

Thanks. Just for interest, do you see any advantage in creating the Namespace object first?

[bob1de]

Server.register_namespace now returns the Namespace instance it created. It then can be used for whatever the user wants. Does this suffice your needs?

[bob1de]

Hm, maybe it is worth thinking about not registering the different handlers in register_namespace(). Instead, the Namespace instance could be stored in Server.handlers[namespace_name]. This way one could implement dynamic event handler mapping using a Namespace.get_event_handler(event_name) method that looks for the requested handler and returns it. Such a method could easily be overloaded by users to map events to handlers dynamically. I think I'm going to implement it, since it is a minor effort with relatively large use.

[bob1de]

This also addresses #41.

By overloading the Server.get_event_handler(event_name, namespace) or Namespace.get_event_handler(event_name) methods one can now easily inject wildcard event handlers for either all or just a specific namespace. This can also be used as a kind of pre-handler that either stops handling or passes on to the real event handler (like for authentication purposes). And it does not conflict with your planned before_event decorator as that will be applied on a per-event handler basis if I understood your comment at miguelgrinberg/Flask-SocketIO#265 and flask's docs correctly.

[bob1de]

Since I needed it right now I implemented the before/after stuff in a quite flexible way. It's in the docs as well under "Event handler middlewares". Even they are not 100% related to class-based namespaces I just added it to this branch, because they are most useful on a per namespace basis. If you generally like it you'll probably want to restructure it in some way, but the basic concept should be visible and is working.

[bob1de]

Oh, I found a bug in this but will fix it later today.

[bob1de]

Well, I think it's ready for revision now. Docs and also tests are written.

[miguelgrinberg]

This PR started well, but now I think it got very complicated. I like the idea of a simple class based mechanism, but I don't want to get so much stuff all in a single PR, makes it much more difficult to review and test. I'm not sure I get the idea of middlewares that you implemented. Seems easier to just follow the class-based approach and add the before and after event handlers in there.

I was going to make a few minor changes to the PR and merge it, but given that the PR got so big, instead I just coded my own version of the class based namespaces, more in the style of your first implementation, but with some improvements that make it easier to document the Namespace class.

[bob1de]

You are right in that it's generally a bad idea to mix two issues into one PR. Sorry for that. I'm not yet too familiar to Github's features. Maybe there is a way to split it after a specific commit? At least I collapsed the commits belonging to a particular issue. Now there are just two commits left that can easily be differenciated.

The second commit deals with the concept of event handler middlewares. I wrote a chapter in the docs about them. Basically it's a way to add before/after event handlers on three layers for fine-grained control: These layers can alter the arguments the event handler will get or its return value transparently or completely abort execution, returning something else.

• at server layer for every event by adding the middleware to socketio.Server().middlewares
• at namespace layer by adding the middleware to socketio.Namespace().middlewares
• at the event handler layer by decorating an event handler with @util.apply_middleware(...)

Finally, when the event is triggered, the socketio.Server()._trigger_event method collects middlewares for the used event handler from these three layers (from least to most specific one) and applies them on the handler before execution.

I wrote a test for it that is commented and it works well.

Hopefully it won't get lost after all because it is so simple and powerful.

EDIT:
Rebased commits to reflect their issue.

[bob1de]

And I added all three layers you can add middlewares at to get the maximum flexibility. The cost in both code complexity and performance is not really higher for three than it is for one.

[bob1de]

I just reviewed what your implementation of class-based namespaces looks like.

The following points are not yet perfect IMHO:

• overwriting event names that should contain characters illegal in method names is not possible (see @Namespace.event_name(...) in my implementation)
• Shouldn't Namespace.set_server be private to make it clear that one should use Server.register_namespace instead?
• Doc string of Server.register_namespace mentions that you can pass either class or instance
• This is a really conceptual question one could discuss about. Why do you use separate dicts for handlers and namespace_handlers? That way you can mix handlers of a Namespace object with those explicitly registered by Server.on for the same namespace. It could be useful for testing but maybe is a bit confusing. I stored both namespace objects and namespace dicts in one single Server.handlers dictionary and let Server._trigger_event decide how to look event handlers up for a given namespace..

Maybe you want to take some of my code to change the mentioned points, or maybe you have reasons you preferred the way you went?

Apart from that it looks nice.

BTW: My middleware implementation would perfectly work on your class-based namespaces as well.

[bob1de]

Now it's even less new code, probably less than 100 lines at all that belongs to middlewares (excluding docs and tests of course). The docs got an update as well to be more detailled. Simply ask when there are questions.

[miguelgrinberg]

overwriting event names that should contain characters illegal in method names is not possible (see @Namespace.event_name(...) in my implementation)

Yes. I documented that to use class based namespaces you have to use events that have legal method names. Having to replace characters in events makes the relationship between events and methods less clear, you can have two events going to the same method, for example. I prefer not to do that.

Shouldn't Namespace.set_server be private to make it clear that one should use Server.register_namespace instead?

It's really not private, since another class is using it. I decided to leave it public, but not document it. It's really not a private method, since another class is using it.

Doc string of Server.register_namespace mentions that you can pass either class or instance

Ah, yes, that is a mistake. I had it that way first, then decided against it. I'll fix it.

This is a really conceptual question one could discuss about. Why do you use separate dicts for handlers and namespace_handlers? That way you can mix handlers of a Namespace object with those explicitly registered by Server.on for the same namespace. It could be useful for testing but maybe is a bit confusing. I stored both namespace objects and namespace dicts in one single Server.handlers dictionary and let Server._trigger_event decide how to look event handlers up for a given namespace..

Because with this you can override an event and have it go to a decorated function, while everything else goes to the class.

[miguelgrinberg]

I have a mostly working implementation of the before and after events that I'm currently testing. I have been working on for a while, based on miguelgrinberg/Flask-SocketIO#265. To be consistent with the existing functionality I wanted these events to be decorator based. I think they can also be integrated with the class-based namespaces, but the main functionality is going to be based on decorators. I still don't see the need to have a separate middleware class. That seems overly complicated.

[bob1de]

I'm however not so happy with the event name restrictions. There are often event names having spaces or dots in them that don't work with the current approach.

I actually got the middlewares separated from the namespaces. Should I open another PR for this that bases on current master?

[bob1de]

And, I think methods starting with an _ are not necessarily meant to be only used from inside the object. I always interpreted them as something that is not part of the official API and thus should not be used from outside the library by users.

http://stackoverflow.com/questions/1301346/the-meaning-of-a-single-and-a-double-underscore-before-an-object-name-in-python

_single_leading_underscore: weak "internal use" indicator. E.g. from M import * does not import objects whose name starts with an underscore.

[miguelgrinberg]

@efficiosoft It's a minor detail though. I changed it to a _ method, I really don't mind either way.

As far as different rules for matching events to methods, I have documented the trigger_event method in the namespace class, you can override it in your Namespace class to do the mapping that you like.

[bob1de]

Yes, that was no criticism, I'm just not always sure how to use the _. I just wanted to hear your thoughts about it. :-)

I'm going to open another PR now for the middlewares. I wrote them compatible to master without conflicts.

[miguelgrinberg]

I think you need to wait until I finish my before and after event support. As I said above, I don't find your idea of middlewares that straightforward, and besides, there is already a middleware in this package (a WSGI one), so the choice of name is also not that great.

[bob1de]

Ok, I'll let it in that state now until you make a decision. I find it quite straightforward actually. It reminds me of the middleware system django provides.

I've written one that applies encoding and decoding with msgpack, for example, in just a few lines of code. I post it in the new PR as an example.

#45