vsgi: Make 'ApplicationCallback' return true if handled

[arteymix]

As described in #165

Introducing this feature has many advantages:

• return next directly
• try next and do something else if it's being ignored
• let the server know if the request has been ignored

However, it's a pretty aggressive API break.

• [ ] handle ignored requests (see what servers can do about it)
• [x] update the documentation (especially the code examples)
• [x] obtain positive feedback from @Bob131

[Bob131]

This is fantastic. Two big thumbs up from me. However, this commit relates to something I've been thinking about for a bit, so wall of text incoming I wanted to suggest exactly this before, but held back thinking that it was incongruous with the existing API style. Something I may or may not have said before (apologies if I'm beating you over the head with it) is how head-of-the-nail perfect some of Flask's API choices are. It turned server-side HTTP stuff from a drag to quite a lot of fun, something I think we should aim to mimic. I've been thinking on this, and I think 'expressivity' is the reason I find it so appealing. Flask employs a liberal sprinkling of sensible defaults in a way that allows you to convey more functionality with less code, which ultimately has a pretty positive effect on clarity. An example I'm particularly fond of relates to what I was saying in #160; in Flask, route building is done with decorators, so the route becomes part of the function declaration. I would consider this commit to make definite improvements in expressivity. However, I would go one further: Rather than returning bool, perhaps we could return `VSGI.Response?`? My thinking on this is rather half-baked, but it seems suboptimal to be passing around that long list of arguments and returning a bool. Like a waste of syntax, if you get my drift. Given we're talking about making big, breaking changes to the VSGI API, I have some ideas that I want to throw out there for you to munch on. They're not necessarily good ideas but just stuff to consider: - Jamming all our bits and bobs into the routing context. The request and the `next` function are certainly what I would describe as contextual, and it allows extra details to be abstracted away in the future. Less arguments == less verbosity as well, which I would consider an improvement - Delegating the responsibility of initialising the `Response` object to the `ApplicationCallback`. This I'm less sure of because I don't know how it impacts the overall design of allowing `ApplicationCallbacks` to be chained, but it has two benefits: - This allows us to polymorphize. Specifically what I'm imagining is say the user wants to respond with a multipart encoded file, or say they want to abstract away some response boilerplate. This could be achieved by subclassing `Response` and returning objects of that type. If the `Response` object mandated that there be an `OutputStream` accessible, then Valum could just read to the client whatever acrobatics our `Response` subtype needed to perform - This eases the concern noted above re "wasted syntax" since functions could just return `Response`s they had initialised - Rather than allowing an arbitrary number of `ApplicationCallback`s to be bound to a route, perhaps we could have a context handler delegate. The idea being that the `ApplicationCallback` gets complete control to do what they like with the response object since that's their responsibility, and if they return one then start calling each context handler before handing off to the client. If I'm honest, I don't really believe a great deal in this idea and would rather allow only a `ApplicationCallback` on a route binding since all the cases I can think of would be covered by a monolithic context and polymorphic `Response`, but in the name of not removing existing functionality this sounds like a reasonable compromise. Sorry for the enormous amount of text. If you so desire, please write me back an equally enormous amount of text about how wrong I am; I've not ever had to maintain a stable API or even work towards versioned releases, so you're the expert here :) PS: I emailed you a couple of days back regarding automatic Valadoc builds. Since it requires secret exchange and doesn't specifically relate to the library, I thought it best to contact you over email. I sent it to the one you have listed on your Github profile

[arteymix]

Actually, this is genius, because we can now forward the result of perform_routing: no need to throw a 404 at its bottom :)

@Bob131 I'm writting an answer to your concerns...

[arteymix]

Now, we have consistent behaviours for 404 and 405 errors. However, this is going to change how famous not found pages have to be implemented, because the 405 must be verified as well.

With 2dee38baa118ee27096bf2d9f02fc6e1c9d03c27, this is going to be very convenient for APIs.

[arteymix]

I would consider this commit to make definite improvements in expressivity. However, I would go one further: Rather than returning bool, perhaps we could return VSGI.Response?? My thinking on this is rather half-baked, but it seems suboptimal to be passing around that long list of arguments and returning a bool. Like a waste of syntax, if you get my drift. Given we're talking about making big, breaking changes to the VSGI API, I have some ideas that I want to throw out there for you to munch on. They're not necessarily good ideas but just stuff to consider:

Returning the response is problematic, because it's not immutable and would force absolutely synchronous manipulations. What I would like to see in Vala, however, is asynchronous delegates. In this context, it would make sense to return the Response object.

Jamming all our bits and bobs into the routing context. The request and the next function are certainly what I would describe as contextual, and it allows extra details to be abstracted away in the future. Less arguments == less verbosity as well, which I would consider an improvement

There's a very nice thing about all the delegate model: type compatibility. You should notice that ApplicationCallback are compatible with NextCallback which are in turn compatible with HandlerCallback. I think that this is essential feature that wouldn't work if we would wrap all the states in a context.

Moreover, if you look at how server-sent events are implemented, the Response is literally replaced by a callback to send events. I think it has its dose of elegance.

Delegating the responsibility of initialising the Response object to the ApplicationCallback. This I'm less sure of because I don't know how it impacts the overall design of allowing ApplicationCallbacks to be chained, but it has two benefits: This allows us to polymorphize. Specifically what I'm imagining is say the user wants to respond with a multipart encoded file, or say they want to abstract away some response boilerplate. This could be achieved by subclassing Response and returning objects of that type. If the Response object mandated that there be an OutputStream accessible, then Valum could just read to the client whatever acrobatics our Response subtype needed to perform

Deriving behaviours of the Request and Response object is performed by composition, just like GIO streams. You might want to read this Wikipedia article about the matter: https://en.wikipedia.org/wiki/Composition_over_inheritance

FilteredRequest and FilteredResponse are designed to simplify the composition process.

Also, this is necessary, because various implementations will have their particularities when dealing with incoming and outcoming data. What would you inherit from?

This eases the concern noted above re "wasted syntax" since functions could just return Responses they had initialised Rather than allowing an arbitrary number of ApplicationCallbacks to be bound to a route, perhaps we could have a context handler delegate. The idea being that the ApplicationCallback gets complete control to do what they like with the response object since that's their responsibility, and if they return one then start calling each context handler before handing off to the client. If I'm honest, I don't really believe a great deal in this idea and would rather allow only a ApplicationCallback on a route binding since all the cases I can think of would be covered by a monolithic context and polymorphic Response, but in the name of not removing existing functionality this sounds like a reasonable compromise.

It just sound like the opposite bottom-top approach.

[arteymix]

Another useful thing: you can know if a subrouter has handled the request and provide a failover

app.rule (Method.ALL, "some-prefix/*", (req, res) => {
    return subrouter.handle (req, res) ? true : next (req, res);
});

[arteymix]

Right now, methods for the Allow header are collected by passing through all routes. It might be a better idea to leave the responsilibity of finding alternative candidates to perform_routing.

[arteymix]

The code will have to be updated in the merge to return the result of expand and expand_utf8.

[arteymix]

We'll have to deal with server fallback in a separate PR/issue.

Possible options:

• stack of application with fallback
• fallback based on the implementation