Problem/Motivation
Drupal 8 includes a nested matcher for filtering routes based on the characteristics of an incoming request. As part of the nested matcher process, several partial matchers are applied to the initial RouteCollection (which can include hundreds of routes in the context of Drupal). One example of partial matcher is the HttpMethodMatcher which looks at the method of the HTTP request and discards any route not supporting that method (routes can specify which method they support in their requirements parameter).
Crell suggested to implement another partial matcher for MIME types specified in the HTTP Accept header. Routes can define requirements which are used in the nested matcher. Here is a couple of routes defining the format they support in an implementation of hook_router_info():
$route = new Route('conneg/test1', array(
'_controller' => '\Drupal\conneg\ConnegControllers::test1_html',
), array(
'_format' => 'html',
));
$collection->add('conneg_1_html', $route);
$route = new Route('conneg/test1', array(
'_controller' => '\Drupal\conneg\ConnegControllers::test1_json',
), array(
'_format' => 'json',
));
$collection->add('conneg_1_json', $route);
Note how both routes share the same path, but they point to a different controller based on the format they require.
Given that several MIME types are usually part of an HTTP request, and each of them can be assigned a quality (aka weight), routes should as a result be given a weight. Symfony does not allow to weigh routes other than using the order they are ->add()'ed to a RouteCollection, so this might require us to use a custom collection first in the nested matcher and then add all routes in the final RouteCollection from that array, in the right order.
Proposed resolution
Implement a new PartialMatcher class and register it in the DIC in such as a way that it is included in the NestedMatcher process.
Remaining tasks
Review approach used in patch.
User interface changes
none.
API changes
New argument '_format' in the requirements of a route (similar to '_method').
Comment | File | Size | Author |
---|---|---|---|
#21 | mimetype-matcher-1833440-21.patch | 2.43 KB | klausi |
#21 | mimetype-matcher-1833440-21-interdiff.txt | 2.08 KB | klausi |
#20 | 1833440_20_mimetype_matcher.patch | 2.45 KB | scor |
#18 | 1833440_18_mimetype_matcher.patch | 2.01 KB | scor |
#13 | routing-mime-1833440-13.patch | 8.45 KB | klausi |
Comments
Comment #0.0
scor CreditAttribution: scor commentedadd note about route order
Comment #1
scor CreditAttribution: scor commentedHere is my initial patch based on discussions during BADCamp with @Crell. For now it implements a crappy 'weak routes' array in place of the ordered route collection where routes which are matching because they don't specify a format or match because of */* are added. These are added to the RouteCollection in the end so they have a lower priority.
TODO: switch to the Symfony mime type groups defined in Request::initializeFormats().
At this point reviews on the general directions are welcome.
Comment #2
Crell CreditAttribution: Crell commentedThis looks good on general direction. Let's go ahead and use the Response class's mapper for now, then we can swap to the new conneg library when that exists.
Comment #3
scor CreditAttribution: scor commentedAdded support for Symfony's built-in formats with associated MIME types, and new test for an alternative JSON Accept header.
RouteNotFoundException will have to be replaced with UnsupportedMediaTypeHttpException once #1831074: Send 415 Unsupported Media Type http header when serializer encounters an unsupported format and #1834594: Update dependencies (Symfony and Twig) follow up fixes for Composer are fixed.
Comment #5
scor CreditAttribution: scor commentedI tried using array_filter() here using Request::getFormat() as callable, but couldn't figure it out. something like:
Comment #6
Crell CreditAttribution: Crell commentedIf you want to use a method of an object as a callable, make it an array:
$acceptable_formats = array_filter($acceptable_mime_types, array($request, 'getFormat'));
Comment #7
Crell CreditAttribution: Crell commentedFortunately the bugs in the previous patch were easy to fix, and I did some other tidying while I was in there. This is passing all of the Routing tests for me locally.
Comment #8
scor CreditAttribution: scor commentedThanks @crell for the fixes. following up with some additional test coverage to keep up with the tests of HttpMethodMatcher
Comment #9
Crell CreditAttribution: Crell commentedIf the bot's happy, I'm happy. Mr. Roboto will tell us if he's not happy.
Comment #11
scor CreditAttribution: scor commentedobviously I should not make supposedly trivial changes without re-running the tests. fixed MimeTypeMatcherTest.php to include the right exception.
Comment #12
Crell CreditAttribution: Crell commentedThank you, Mr. Roboto.
Comment #13
klausiLooks good to me as well, made some minor documentation fixes.
Comment #14
klausiFollow-up for REST module: #1846582: Restrict data retrieving routes to media type
Comment #15
Crell CreditAttribution: Crell commentedSince this is a blocker for the full on REST support, I'm bumping priority.
Comment #16
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.
Comment #17
scor CreditAttribution: scor commentedFollow up issue for finer partial matcher: #1844920: Add weight to routes
Comment #18
scor CreditAttribution: scor commentedFollow up patch waiting on #1831074: Send 415 Unsupported Media Type http header when serializer encounters an unsupported format and #1810472: Add Symfony's Serializer component to core despite Symfony potentially releasing BC-breaking updates after 2.3..
Comment #19
scor CreditAttribution: scor commentedGiven that #1834594: Update dependencies (Symfony and Twig) follow up fixes for Composer was committed, this patch can now be tested/reviewed.
Comment #20
scor CreditAttribution: scor commentedthis patch also removes the @todo comment :)
Comment #21
klausiImproved use statement ordering, removed unused use statements.
Comment #22
Crell CreditAttribution: Crell commentedYay, correct exceptions.
Comment #23
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.
Comment #24.0
(not verified) CreditAttribution: commentedinput format quirk