search

valum-framework / valum

Web micro-framework written in Vala
https://valum-framework.readthedocs.io/en/latest/
GNU Lesser General Public License v3.0
225 stars 23 forks source link

Improve content negociation middlewares #150

Closed arteymix closed 8 years ago

arteymix commented 8 years ago

In short, it handles fuzzy matching and set the appropriate header in the response.

using Valum.ContentNegotiation;

app.get ("", accept ("text/html", (req, res) => {
    res.body.write_all ("<!DOCTYPE html><html></html>", null);
})).then (accept ("text/xhtml", (req, res) => {
    res.body.write_all ("...");
}, NegociateFlags.FINAL));

The NegociateFlags.FINAL indicate that this is the last option and a 406 Not Acceptable should be raised on failure.

It needs some tests before being merged.

arteymix commented 8 years ago

It would be great to have something that allow us to build sequences and connect the last next to the keep routing after the sequence.

Something like:

app.get ("", sequence(
    accept ("text/xml", produce_xml),
    accept ("text/html", produce_html, NegociateFlags.FINAL)
));

That could get rid of then.

arteymix commented 8 years ago

It would be great to handle quality values in some way.

arteymix commented 8 years ago

To handle quality values properly, we could provide multiple choices to negotiate and let it forward the best possible:

accept ({"text/html", "text/html+xml"}, (req, res, next, stack, content_type) {
    // ...
}};
arteymix commented 8 years ago

The tests work, but it produces a SEGFAULT when running the application :(

arteymix commented 8 years ago

Okay, everything is correct now!

Since we handle multiple expectations, the default behaviour is to raise a 406 Not Acceptable, which can be overwritten by setting the NegotiateFlags.NEXT.

No expectations always raise a 406 Not Acceptable and a missing header forwards the first expectation.

We need some tests for the edge cases and I have some documentation already ;)

arteymix commented 8 years ago

The tests have to cover the use of mixed character case, because most of the specification is case-insensitive.

arteymix commented 8 years ago

The merge will have to deal with a couple of breaking changes:

arteymix commented 8 years ago

Just picked the codecov commit, so that we could see what parts are missing. It should end-up here: https://codecov.io/github/valum-framework/valum?pr=150

arteymix commented 8 years ago

It would be nice to use a quality list for the expectations and select the best match by taking the highest qvalue product. This is what Apache uses.

app.get ("/", accept ("text/json; q=1, text/xml; q=0.1", (req, res, next, ctx, expectation) => {
    // produce content according to 'expectation'
}));
arteymix commented 8 years ago

I merged the changes from the 0.3 so it should be fast-forward into the trunk.

Some tests are missing and it should be ready to merge!

codecov-io commented 8 years ago

Current coverage is 63.28%

Merging #150 into master will increase coverage by +2.11%

@@             master       #150   diff @@
==========================================
  Files            28         28          
  Lines          1025       1084    +59   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            627        686    +59   
  Misses          398        398          
  Partials          0          0
  1. File ...i/vsgi-response.vala was modified. more
    • Misses -2
    • Partials 0
    • Hits +2

Powered by Codecov. Last updated by b7870a2...42c1ab4