Restrict data retrieving routes to media type

+++ b/core/modules/jsonld/lib/Drupal/jsonld/EventSubscriber/JsonldSubscriber.php
@@ -0,0 +1,41 @@
+  public function onKernelRequest(GetResponseEvent $event) {
+    $event->getRequest()->setFormat('drupal_jsonld', 'application/vnd.drupal.ld+json');
+    $event->getRequest()->setFormat('jsonld', 'application/ld+json');
+  }

Micro-optimization: Only call getRequest() once and just call setFormat() on that twice. (Someone should file an issue upstream to make setFormat() chainable...)

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/ResourceBase.php
@@ -55,25 +55,46 @@ public function routes() {
+        // Restrict GET and HEAD requests to the media type specified in the
+        // HTTP Accept headers.
+        if ($method == 'GET' || $method == 'HEAD') {

Why are only GET/HEAD routes restricted? Shouldn't PUT and POST also be Accept-aware?

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/ResourceBase.php
@@ -55,25 +55,46 @@ public function routes() {
+          $route = new Route("/$prefix/{id}", array(
+            '_controller' => 'Drupal\rest\RequestHandler::handle',
+            // @todo Once http://drupal.org/node/1793520 is committed we will have
+            // route object avaialble in the controller so 'plugin' property
+            // should be changed to '_plugin'.
+            // @see RequestHandler::handle().
+            'plugin' => $this->plugin_id,
+          ), array(
+            // The HTTP method is a requirement for this route.
+            '_method' => $method,
+            '_permission' => "restful $lower_method $this->plugin_id",
+            // The media type format is a requirement, too.
+            // @todo Replace hard coded format here with available formats.
+            '_format' => 'drupal_jsonld',
+          ));
+        }
+        else {
+          $route = new Route("/$prefix/{id}", array(
+            '_controller' => 'Drupal\rest\RequestHandler::handle',
+            // @todo Once http://drupal.org/node/1793520 is committed we will have
+            // route object avaialble in the controller so 'plugin' property
+            // should be changed to '_plugin'.
+            // @see RequestHandler::handle().
+            'plugin' => $this->plugin_id,
+          ), array(
+            // The HTTP method is a requirement for this route.
+            '_method' => $method,
+            '_permission' => "restful $lower_method $this->plugin_id",
+          ));

It looks like the only difference between these two sections is the _format requirement. That's easy to add via ->addRequirement(), so let's eliminate the duplication here and just wrap the if statement around that.

I reran the tests locally and get the same errors. When run locally, there is a Verbose message which gives the Fatal Error. Hope this helps:

D #227 (Previous | Next)
POST request to: admin/modules
Ending URL: http://d/d8x/admin/modules
Fields: <?php array (
  'modules[Core][breakpoint][enable]' => '1',
  'modules[Core][comment][enable]' => '1',
  'modules[Core][dblog][enable]' => '1',
  'modules[Core][history][enable]' => '1',
  'modules[Core][image][enable]' => '1',
  'modules[Core][jsonld][enable]' => '1',
  'modules[Core][language][enable]' => '1',
  'modules[Core][search][enable]' => '1',
  'modules[Core][taxonomy][enable]' => '1',
  'form_build_id' => 'form-ZGs-sC69Mu8GghgOpXp_I9pUn3Np5xr8ATpfmO3nvXk',
  'form_token' => 'QwOiwOmNv_ejgh6NqO_3LyZ5nFTZqv6Qn7c1odIZtSg',
  'form_id' => 'system_modules',
  'op' => 'Save configuration',
)

( ! ) Fatal error: Class 'Drupal\jsonld\EventSubscriber\JsonldSubscriber' not found in /Users/mweitzman/htd/d8x/core/vendor/symfony/event-dispatcher/Symfony/Component/EventDispatcher/ContainerAwareEventDispatcher.php on line 140
Call Stack
#	Time	Memory	Function	Location
1	0.0003	632856	{main}( )	../index.php:0
2	0.0018	825160	Drupal\Core\DrupalKernel->boot( )	../index.php:35
3	0.0018	825160	Drupal\Core\DrupalKernel->initializeContainer( )	../DrupalKernel.php:145
4	0.0033	1181624	Symfony\Component\DependencyInjection\Container->get( )	../DrupalKernel.php:296
5	0.0034	1182216	service_container_prod__simpletest810437->getConfig_FactoryService( )	../Container.php:262
6	0.0039	1236184	Symfony\Component\DependencyInjection\Container->get( )	../6de9219f8bdaee23ef71cc6af0f7884c545c9f07de2d12a245808238c9bf1e74.php:186
7	0.0039	1236552	service_container_prod__simpletest810437->getDispatcherService( )	../Container.php:262
8	0.0055	1321704	Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher->addSubscriberService( )	../6de9219f8bdaee23ef71cc6af0f7884c545c9f07de2d12a245808238c9bf1e74.php:305

This patch had the unlucky fate of exposing an existing bug in core, simply in virtue of its bringing into existence for the first time a module that provides an event subscriber and is required by another module. To see that this is the circumstance under which the failures occur, make ban module (which also provides an event subscriber) a requirement of some other module and watch this test explode in a similar manner.

@klausi, I should have spotted this when you asked me about it the other day, because the truth is I had come across this bug in the extension handler patch and fixed it there, but didn't put two and two together. The short explanation of why this happens is that the container.modules parameter doesn't get set properly during module_enable(), but the only time we even use that parameter is when we register module namespaces in the kernel before it gets the list of enabled modules from the config service - and the very act of looking up a config object causes an event to be triggered... but the module namespaces for the event subscribers have not been registered yet.

This should fix it. I also addressed @Crell's points in #11 (hopefully correctly!) apart from the question about PUT/POST because I don't know the answer.

Oh, what did not get captured in the interdiff is the fact that I left out the changes to core/lib/Drupal/Core/Routing/MimeTypeMatcher.php and core/modules/system/lib/Drupal/system/Tests/Routing/MimeTypeMatcherTest.php because it looks like they were solved already in a different issue - is that correct?

@katbailey yes, the hunks of the 2 MimeTypeMatcher files have been committed in the meantime.

Nice teamwork, folks.

Committed to 8.x. Thanks!

What was this change for?

diff --git a/core/includes/module.inc b/core/includes/module.inc
index abe03dd..9be96cd 100644
--- a/core/includes/module.inc
+++ b/core/includes/module.inc
@@ -493,6 +493,7 @@ function module_enable($module_list, $enable_dependencies = TRUE) {
   $schema_store = drupal_container()->get('keyvalue')->get('system.schema');
   $module_config = config('system.module');
   $disabled_config = config('system.module.disabled');
+  $module_filenames = drupal_container()->getParameter('container.modules');
   foreach ($module_list as $module) {
     // Only process modules that are not already enabled.
     $enabled = $module_config->get("enabled.$module") !== NULL;
@@ -516,11 +517,12 @@ function module_enable($module_list, $enable_dependencies = TRUE) {
       system_list_reset();
       module_implements_reset();
       _system_update_bootstrap_status();
+      $module_filenames[$module] = drupal_get_filename('module', $module);
       // Update the kernel to include it.
       // @todo The if statement is here because install_begin_request() creates
       //   a container without a kernel. It probably shouldn't.
       if ($kernel = drupal_container()->get('kernel', ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
-        $kernel->updateModules(module_list(), array($module => drupal_get_filename('module', $module)));
+        $kernel->updateModules(module_list(), $module_filenames);
       }
       // Refresh the schema to include it.

@catch see #13 for a not very in-depth explanation of why this was needed - let me know if you need more info. Is the change problematic?