Issue #3067762 by alexpott, Berdir, Wim Leers: Fix bug in...

Issue #3067762 by alexpott, Berdir, Wim Leers: Fix bug in \Drupal\rest\RequestHandler::createArgumentResolver and handle "Passing in arguments the legacy way is deprecated" deprecation

core/modules/rest/src/RequestHandler.php

@@ -284,9 +284,15 @@ protected function createArgumentResolver(RouteMatchInterface $route_match, $uns
     }
 
     if (in_array($request->getMethod(), ['PATCH', 'POST'], TRUE)) {
-      $upcasted_route_arguments['entity'] = $unserialized;
-      $upcasted_route_arguments['data'] = $unserialized;
-      $upcasted_route_arguments['unserialized'] = $unserialized;
+      if (is_object($unserialized)) {
+        $upcasted_route_arguments['entity'] = $unserialized;
+        $upcasted_route_arguments['data'] = $unserialized;
+        $upcasted_route_arguments['unserialized'] = $unserialized;
+      }
+      else {
+        $raw_route_arguments['data'] = $unserialized;
+        $raw_route_arguments['unserialized'] = $unserialized;
+      }
       $upcasted_route_arguments['original_entity'] = $route_arguments_entity;
     }
     else {

core/modules/rest/tests/src/Kernel/RequestHandlerTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Tests\rest\Kernel;
 
+use Drupal\Component\Serialization\Json;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Config\ImmutableConfig;
 use Drupal\Core\Routing\RouteMatch;
@@ -10,8 +11,10 @@
 use Drupal\rest\RequestHandler;
 use Drupal\rest\ResourceResponse;
 use Drupal\rest\RestResourceConfigInterface;
+use Prophecy\Argument;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Route;
+use Symfony\Component\Serializer\Encoder\DecoderInterface;
 use Symfony\Component\Serializer\SerializerInterface;
 
 /**
@@ -45,6 +48,9 @@ public function setUp() {
     $config_factory->get('rest.settings')
       ->willReturn($this->prophesize(ImmutableConfig::class)->reveal());
     $serializer = $this->prophesize(SerializerInterface::class);
+    $serializer->willImplement(DecoderInterface::class);
+    $serializer->decode(Json::encode(['this is an array']), NULL, Argument::type('array'))
+      ->willReturn(['this is an array']);
     $this->requestHandler = new RequestHandler($config_factory->reveal(), $serializer->reveal());
   }
 
@@ -52,6 +58,53 @@ public function setUp() {
    * @covers ::handle
    */
   public function testHandle() {
+    $request = new Request([], [], [], [], [], [], Json::encode(['this is an array']));
+    $route_match = new RouteMatch('test', (new Route('/rest/test', ['_rest_resource_config' => 'restplugin', 'example' => ''], ['_format' => 'json']))->setMethods(['GET']));
+
+    $resource = $this->prophesize(StubRequestHandlerResourcePlugin::class);
+    $resource->get('', $request)
+      ->shouldBeCalled();
+    $resource->getPluginDefinition()
+      ->willReturn([])
+      ->shouldBeCalled();
+
+    // Setup the configuration.
+    $config = $this->prophesize(RestResourceConfigInterface::class);
+    $config->getResourcePlugin()->willReturn($resource->reveal());
+    $config->getCacheContexts()->willReturn([]);
+    $config->getCacheTags()->willReturn([]);
+    $config->getCacheMaxAge()->willReturn(12);
+
+    // Response returns NULL this time because response from plugin is not
+    // a ResourceResponse so it is passed through directly.
+    $response = $this->requestHandler->handle($route_match, $request, $config->reveal());
+    $this->assertEquals(NULL, $response);
+
+    // Response will return a ResourceResponse this time.
+    $response = new ResourceResponse([]);
+    $resource->get(NULL, $request)
+      ->willReturn($response);
+    $handler_response = $this->requestHandler->handle($route_match, $request, $config->reveal());
+    $this->assertEquals($response, $handler_response);
+
+    // We will call the patch method this time.
+    $route_match = new RouteMatch('test', (new Route('/rest/test', ['_rest_resource_config' => 'restplugin', 'example_original' => ''], ['_content_type_format' => 'json']))->setMethods(['PATCH']));
+    $request->setMethod('PATCH');
+    $response = new ResourceResponse([]);
+    $resource->patch(['this is an array'], $request)
+      ->shouldBeCalledTimes(1)
+      ->willReturn($response);
+    $handler_response = $this->requestHandler->handle($route_match, $request, $config->reveal());
+    $this->assertEquals($response, $handler_response);
+  }
+
+  /**
+   * @covers ::handle
+   * @covers ::getLegacyParameters
+   * @expectedDeprecation Passing in arguments the legacy way is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Provide the right parameter names in the method, similar to controllers. See https://www.drupal.org/node/2894819
+   * @group legacy
+   */
+  public function testHandleLegacy() {
     $request = new Request();
     $route_match = new RouteMatch('test', (new Route('/rest/test', ['_rest_resource_config' => 'restplugin'], ['_format' => 'json']))->setMethods(['GET']));
 
@@ -100,7 +153,7 @@ public function get($example, Request $request) {}
 
   public function post() {}
 
-  public function patch($example_original, Request $request) {}
+  public function patch($data, Request $request) {}
 
   public function delete() {}
 

core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php

@@ -140,7 +140,6 @@ public static function isDeprecationSkipped($message) {
    */
   public static function getSkippedDeprecations() {
     return [
-      'Passing in arguments the legacy way is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Provide the right parameter names in the method, similar to controllers. See https://www.drupal.org/node/2894819',
       'The Symfony\Component\ClassLoader\ApcClassLoader class is deprecated since Symfony 3.3 and will be removed in 4.0. Use `composer install --apcu-autoloader` instead.',
       // The following deprecation is not triggered by DrupalCI testing since it
       // is a Windows only deprecation. Remove when core no longer uses