A bit restricting Middleware interface?

[ciscoheat]

Hi, this looks great, can't wait to try monsoon for the next project! Only one thing, would it be possible to have a middleware interface like the process method you have in two of the bundled ones? In case you don't want the constructor restricted, and don't need to use the router.

[benmerckx]

Sure! Could you give me an example of how you'd use it? Or what the interface should like? Is it something like this you're looking for, which would run for every request?

interface ProcessMiddleware {public function process(request: Request, response: Response): Void;}

[ciscoheat]

Yes that would be fine, I'm thinking about some frequently used, global middleware like gzip compression, or authentication.

I wonder though, is it a good idea to mix the router and middleware concepts? I would have thought they were different, something like:

// Create some routes/a controller
interface Router {public function createRoutes(router : Router) : Void;}

// Check the request/response
interface Middleware {public function process(request: Request, response: Response): Void;}

(Perhaps they could even be typedefs?)

When would it be useful to mix them? I may not know enough of your ideas. :) Anyway my main gripe is the constructor typedef. I wouldn't want to limit people's ability to create their own objects.

[benmerckx]

The distinction you make does make more sense, and the naming will make it easier to understand. I was not happy with the current situation but it worked for now.

There's a few reasons I started with the constructor typedef, being:

• Simple api: app.route(MyMiddleware)
  This would change to app.route(new MyMiddleware()), not much of a problem really
• Lazy instantiation, on php/mod_neko the middleware/controllers don't have to be created for each request. So app.route(['/blog' => Blog]) would only be created if /blog is matched in the first place. Thinking about this now this is probably not a valid concern as at most you'd add some routes which overall won't make much of a difference.
• I liked the idea of having all routes defined in the constructor, which is usually placed at the top and would make it easy to find an overview of what the controller is responsible for. I think I'll just place the createRoutes method first then :)

The concepts got mixed a bit because, for example, the static middleware needs to define a route. I chose not the change the request's path, where expressjs does. For example app.route('/assets', Static.serve('public')) can get the correct path (stripped of /assets) by using a splat in the route. Which means going forward I will need to either store the current path in the request, or implement the Static middleware as Router which would mix the concepts again.

Would you say then that it's also better to seperate the concepts by making a distinction like in express with app.route (for Router) and app.use (for Middleware)? I felt like they do mostly the same, except for passing a next callback. I tried to merge these by placing the next callback on the request. Which means you can always pass to the next route, which makes sense in some complex routing scenarios (where you need to check something first and potentially pass to the next route).

I'll implement the interfaces you proposed as typedefs in the next version (I also think there's no real need for them to be interfaces).

[ciscoheat]

Sounds really good, thanks for considering! :) Naming is always tricky, so what do you think of the interface methods? Do they convey correct intent and meaning? There's a name conflict in the Router, and maybe the method process is a bit too generic, but on the other hand the (request, response) signature is quite well known. processRequest maybe... you decide. :)

I think it would be a very nice solution if the Static middleware implemented both Router and Middleware, if it's required. The interfaces would complement each other in a useful class, I like it!

About the separation as done in express, I'd be very happy if you followed express conventions where it's suitable. A familiar feeling is nice.

Interfaces vs. typedefs is interesting too... I've just been bitten by typedefs actually, I came back to a project after some time, wanted to know what types was a certain typedef, and I realized it was quite difficult to find them all. But interfaces requires classes, which some people might not like compared to:

typedef Middleware = Request -> Response -> Void;
typedef Router = Router -> Void; // Oops, naming problem

What do you think about that solution?

[benmerckx]

As for the typedefs, I initially thought you meant something like this:

typedef Router = {function createRoutes(router : Router) : Void;}
typedef Middleware = {function process(request: Request, response: Response): Void;}

I'll decide on whether we need an app.use while updating everything. There were some more reasons for omitting it which will probably come back to me as I have a closer look at the architecture.

Router might be better of as Controller even though the meaning has been clouded by every other library out there. Still, in a web framework it might convey the right meaning.

[ciscoheat]

I had those typedefs in mind originally, just got thinking about whether it's possible to make it even simpler. Many of the examples you've made uses anonymous functions, so could it be an idea to keep doing that even for extensions?

Totally agree with Controller having a diffuse meaning nowadays! Have you read my Rediscovering MVC article? https://github.com/ciscoheat/mithril-hx/wiki/Rediscovering-MVC

[benmerckx]

Well, it should be possible except for injecting the middleware into a callback, as I do for the Body middleware: https://github.com/benmerckx/monsoon#body Thinking about it, that was probably the main reason for the constructor typedef :) I'll have to take a bit more time to think this over as I don't want to lose that functionality. I envision a few more uses for this, and it felt like an ideal way of solving those.

I have read it before, but not in depth, I'll get to that!

[benmerckx]

After giving it some thought here's what I propose:

A new app.use method (similar to express) which takes either a Request -> Response -> Void or an object which unifies with the Middleware typedef defined above (with the process method). The use method acts differently from 'normal' routing in two cases (also similar to express):

• The path matched will also match anything after it: app.use('/test', mw) is the same as app.route('/test/*', mw), and app.use(mw) is the same as app.route('*', mw)
• The request.path changes with every level of routing done in app.use. Which means in app.use('/test', mw) the /test will be stripped from the path. I'll add a fullPath property in case it's needed.

Middleware can be injected in a route if it unifies with the Middleware typedef and has a constructor with no arguments. I can't see a way around that. Maybe someday the constructor parameters can be injected as well through metadata but I don't see the use for now.

Defining more routes can be done through either a Router -> Void or

typedef RouteController = {function createRoutes(router : Router) : Void;}

These can be used in app.use as well.

[ciscoheat]

Sorry for not getting back earlier, but it sounds like you have a plan! The middleware injection is tricky, I was thinking about some factory or DI for that, perhaps... Just brainstorming:

app.injectMiddleware(Body, function(environment) return new Body(environment.something));

[benmerckx]

It's already getting there. But since this revision changes quite a few things internally I'll need to focus on the tests first before releasing it. The current tests are a quick solution and I need a something better, I'm going to have a go at setting up travix and buddy. What you propose is quite clever! Where would environment come from though?

[ciscoheat]

Travix is great! I switched to it for almost all my projects, it's so painless. And you know I like buddy of course. :) If you like, send me a mail (ciscoheat at gmail) and I can help you get started.

The environment could be coming from Sys, so maybe it's not required as an argument. It's still nice to have something passed into the factory function instead of static access, if we can figure out what's available and useful at runtime... I don't know if request and response are useful at that level of initialization, but if it is, it starts to look similar to a Middleware:

typedef MiddlewareInjector<T> = Request -> Response -> T;

[benmerckx]

Thanks for the offer, I had already used both in await so set up went well. I did discover the ConsoleColorReporter in buddy which makes things more fun :)

I've fixed the failing tests (except for one cpp bug) and updated the readme. I've released the update on haxelib as 0.1.0. I plan to rewrite some parts next (pathmatcher and the route macros, which should become a lot simpler with app.use now) and fix all open todos. Afterwards I'll take a look at proper injection of Middleware.