Issue #2506533 by Jaesin, minnur, neclimdul, sdstyles, dawehner, Crell: Remove ContentNegotiation and embed functionality in the middleware

core/core.services.yml

@@ -630,12 +630,8 @@ services:
   http_kernel.basic:
     class: Symfony\Component\HttpKernel\HttpKernel
     arguments: ['@event_dispatcher', '@controller_resolver', '@request_stack']
-  http_negotiation.format_negotiator:
-    class: Drupal\Core\ContentNegotiation
-    private: true
   http_middleware.negotiation:
     class: Drupal\Core\StackMiddleware\NegotiationMiddleware
-    arguments: ['@http_negotiation.format_negotiator']
     tags:
       - { name: http_middleware, priority: 400 }
   http_middleware.reverse_proxy:

core/lib/Drupal/Core/ContentNegotiation.php

@@ -1,43 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\Core\ContentNegotiation.
- */
-
-namespace Drupal\Core;
-
-use Symfony\Component\HttpFoundation\Request;
-
-/**
- * Provides content negotation based upon query parameters.
- */
-class ContentNegotiation {
-
-  /**
-   * Gets the normalized type of a request.
-   *
-   * The normalized type is a short, lowercase version of the format, such as
-   * 'html', 'json' or 'atom'.
-   *
-   * @param \Symfony\Component\HttpFoundation\Request $request
-   *   The request object from which to extract the content type.
-   *
-   * @return string
-   *   The normalized type of a given request.
-   */
-  public function getContentType(Request $request) {
-    // AJAX iframe uploads need special handling, because they contain a JSON
-    // response wrapped in <textarea>.
-    if ($request->get('ajax_iframe_upload', FALSE)) {
-      return 'iframeupload';
-    }
-
-    if ($request->query->has('_format')) {
-      return $request->query->get('_format');
-    }
-
-    // Do HTML last so that it always wins.
-    return 'html';
-  }
-}

core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php

@@ -7,8 +7,6 @@
 
 namespace Drupal\Core\StackMiddleware;
 
-use Drupal\Core\ContentNegotiation;
-use Drupal\Core\ContentNegotiationInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 
@@ -26,13 +24,6 @@ class NegotiationMiddleware implements HttpKernelInterface {
    */
   protected $app;
 
-  /**
-   * The content negotiator.
-   *
-   * @var \Drupal\Core\ContentNegotiation
-   */
-  protected $negotiator;
-
   /**
    * Contains a hashmap of format as key and mimetype as value.
    *
@@ -45,12 +36,9 @@ class NegotiationMiddleware implements HttpKernelInterface {
    *
    * @param \Symfony\Component\HttpKernel\HttpKernelInterface $app
    *   The wrapper HTTP kernel
-   * @param \Drupal\Core\ContentNegotiation $negotiator
-   *   The content negotiator.
    */
-  public function __construct(HttpKernelInterface $app, ContentNegotiation $negotiator) {
+  public function __construct(HttpKernelInterface $app) {
     $this->app = $app;
-    $this->negotiator = $negotiator;
   }
 
   /**
@@ -63,7 +51,7 @@ class NegotiationMiddleware implements HttpKernelInterface {
     }
 
     // Determine the request format using the negotiator.
-    $request->setRequestFormat($this->negotiator->getContentType($request));
+    $request->setRequestFormat($this->getContentType($request));
     return $this->app->handle($request, $type, $catch);
   }
 
@@ -82,4 +70,31 @@ class NegotiationMiddleware implements HttpKernelInterface {
     return $this;
   }
 
+  /**
+   * Gets the normalized type of a request.
+   *
+   * The normalized type is a short, lowercase version of the format, such as
+   * 'html', 'json' or 'atom'.
+   *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The request object from which to extract the content type.
+   *
+   * @return string
+   *   The normalized type of a given request.
+   */
+  protected function getContentType(Request $request) {
+    // AJAX iframe uploads need special handling, because they contain a JSON
+    // response wrapped in <textarea>.
+    if ($request->get('ajax_iframe_upload', FALSE)) {
+      return 'iframeupload';
+    }
+
+    if ($request->query->has('_format')) {
+      return $request->query->get('_format');
+    }
+
+    // Do HTML last so that it always wins.
+    return 'html';
+  }
+
 }

core/tests/Drupal/Tests/Core/ContentNegotiationTest.php

@@ -1,79 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\Tests\Core\ContentNegotiationTest.
- */
-
-namespace Drupal\Tests\Core;
-
-use Drupal\Core\ContentNegotiation;
-use Drupal\Tests\UnitTestCase;
-use Symfony\Component\HttpFoundation\Request;
-
-/**
- * @coversDefaultClass \Drupal\Core\ContentNegotiation
- * @group ContentNegotiation
- */
-class ContentNegotiationTest extends UnitTestCase {
-
-  /**
-   * @var \Drupal\Core\ContentNegotiation
-   */
-  protected $contentNegotiation;
-
-  /**
-   * {@inheritdoc}
-   */
-  protected function setUp() {
-    parent::setUp();
-
-    $this->contentNegotiation = new ContentNegotiation;
-  }
-
-  /**
-   * Tests the getContentType() method with AJAX iframe upload.
-   *
-   * @covers ::getContentType
-   */
-  public function testAjaxIframeUpload() {
-    $request = new Request();
-    $request->attributes->set('ajax_iframe_upload', '1');
-
-    $this->assertSame('iframeupload', $this->contentNegotiation->getContentType($request));
-  }
-
-  /**
-   * Tests the specifying a format via query parameters gets used.
-   */
-  public function testFormatViaQueryParameter() {
-    $request = new Request();
-    $request->query->set('_format', 'bob');
-
-    $this->assertSame('bob', $this->contentNegotiation->getContentType($request));
-  }
-
-  /**
-   * Tests the getContentType() method when no priority format is found.
-   *
-   * @covers ::getContentType
-   */
-  public function testUnknowContentTypeReturnsHtmlByDefault() {
-    $request = new Request();
-
-    $this->assertSame('html', $this->contentNegotiation->getContentType($request));
-  }
-
-  /**
-   * Tests the getContentType() method when no priority format is found but it's an AJAX request.
-   *
-   * @covers ::getContentType
-   */
-  public function testUnknowContentTypeButAjaxRequest() {
-    $request = new Request();
-    $request->headers->set('X-Requested-With', 'XMLHttpRequest');
-
-    $this->assertSame('html', $this->contentNegotiation->getContentType($request));
-  }
-
-}

core/tests/Drupal/Tests/Core/StackMiddleware/NegotiationMiddlewareTest.php

@@ -0,0 +1,142 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\StackMiddleware\NegotiationMiddlewareTest.
+ */
+
+namespace Drupal\Tests\Core\StackMiddleware;
+
+use Drupal\Core\StackMiddleware\NegotiationMiddleware;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\ParameterBag;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * @coversDefaultClass \Drupal\Core\StackMiddleware\NegotiationMiddleware
+ * @group NegotiationMiddleware
+ */
+class NegotiationMiddlewareTest extends UnitTestCase {
+
+  /**
+   * @var \Symfony\Component\HttpKernel\HttpKernelInterface
+   */
+  protected $app;
+
+  /**
+   * @var \Drupal\Tests\Core\StackMiddleware\StubNegotiationMiddleware
+   */
+  protected $contentNegotiation;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->app = $this->prophesize(HttpKernelInterface::class);
+    $this->contentNegotiation = new StubNegotiationMiddleware($this->app->reveal());
+  }
+
+  /**
+   * Tests the getContentType() method with AJAX iframe upload.
+   *
+   * @covers ::getContentType
+   */
+  public function testAjaxIframeUpload() {
+    $request = new Request();
+    $request->attributes->set('ajax_iframe_upload', '1');
+
+    $this->assertSame('iframeupload', $this->contentNegotiation->getContentType($request));
+  }
+
+  /**
+   * Tests the specifying a format via query parameters gets used.
+   *
+   * @covers ::getContentType
+   */
+  public function testFormatViaQueryParameter() {
+    $request = new Request();
+    $request->query->set('_format', 'bob');
+
+    $this->assertSame('bob', $this->contentNegotiation->getContentType($request));
+  }
+
+  /**
+   * Tests the getContentType() method when no priority format is found.
+   *
+   * @covers ::getContentType
+   */
+  public function testUnknowContentTypeReturnsHtmlByDefault() {
+    $request = new Request();
+
+    $this->assertSame('html', $this->contentNegotiation->getContentType($request));
+  }
+
+  /**
+   * Tests the getContentType() method when no priority format is found but it's an AJAX request.
+   *
+   * @covers ::getContentType
+   */
+  public function testUnknowContentTypeButAjaxRequest() {
+    $request = new Request();
+    $request->headers->set('X-Requested-With', 'XMLHttpRequest');
+
+    $this->assertSame('html', $this->contentNegotiation->getContentType($request));
+  }
+
+  /**
+   * Test that handle() correctly hands off to sub application.
+   *
+   * @covers ::handle
+   */
+  public function testHandle() {
+    $request = $this->prophesize(Request::class);
+
+    // Default empty format list should not set any formats.
+    $request->setFormat()->shouldNotBeCalled();
+
+    // Request format will be set with default format.
+    $request->setRequestFormat('html')->shouldBeCalled();
+
+    // Some getContentType calls we don't really care about but have to mock.
+    $request->get('ajax_iframe_upload', false)->shouldBeCalled();
+    $request_mock = $request->reveal();
+    $request_mock->query = new ParameterBag([]);
+
+    // Calling kernel app with default arguments.
+    $this->app->handle($request_mock, HttpKernelInterface::MASTER_REQUEST, TRUE)
+      ->shouldBeCalled();
+    $this->contentNegotiation->handle($request_mock);
+    // Calling kernel app with specified arguments.
+    $this->app->handle($request_mock, HttpKernelInterface::SUB_REQUEST, FALSE)
+      ->shouldBeCalled();
+    $this->contentNegotiation->handle($request_mock, HttpKernelInterface::SUB_REQUEST, FALSE);
+  }
+
+  /**
+   * @covers ::registerFormat
+   */
+  public function testSetFormat() {
+    $request = $this->prophesize(Request::class);
+
+    // Default empty format list should not set any formats.
+    $request->setFormat('david', 'geeky/david')->shouldBeCalled();
+
+    // Some calls we don't care about.
+    $request->setRequestFormat('html')->shouldBeCalled();
+    $request->get('ajax_iframe_upload', false)->shouldBeCalled();
+    $request_mock = $request->reveal();
+    $request_mock->query = new ParameterBag([]);
+
+    // Trigger handle.
+    $this->contentNegotiation->registerFormat('david', 'geeky/david');
+    $this->contentNegotiation->handle($request_mock);
+  }
+
+}
+
+class StubNegotiationMiddleware extends NegotiationMiddleware {
+  public function getContentType(Request $request) { return parent::getContentType($request); }
+}