From f51b5fb91785073deb23a6ecf0be0e573e39a775 Mon Sep 17 00:00:00 2001 From: Paul Mitchum Date: Wed, 24 Jul 2024 12:56:44 -0500 Subject: [PATCH] Use Request in harvest WebServiceApi (#4235) --- modules/harvest/src/WebServiceApi.php | 36 ++++++------- .../tests/src/Kernel/WebServiceApiTest.php | 53 +++++++++---------- .../tests/src/Unit/WebServiceApiTest.php | 13 ++--- 3 files changed, 47 insertions(+), 55 deletions(-) diff --git a/modules/harvest/src/WebServiceApi.php b/modules/harvest/src/WebServiceApi.php index 7f6ef252fe..b2665727b9 100644 --- a/modules/harvest/src/WebServiceApi.php +++ b/modules/harvest/src/WebServiceApi.php @@ -5,7 +5,7 @@ use Drupal\Core\DependencyInjection\ContainerInjectionInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\JsonResponse; -use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Request; /** * Harvest API controller. @@ -17,13 +17,6 @@ class WebServiceApi implements ContainerInjectionInterface { private const DEFAULT_HEADERS = ['Access-Control-Allow-Origin' => '*']; - /** - * Request stack. - * - * @var \Symfony\Component\HttpFoundation\RequestStack - */ - private $requestStack; - /** * Harvest. * @@ -36,7 +29,6 @@ class WebServiceApi implements ContainerInjectionInterface { */ public static function create(ContainerInterface $container) { return new static( - $container->get('request_stack'), $container->get('dkan.harvest.service') ); } @@ -44,8 +36,7 @@ public static function create(ContainerInterface $container) { /** * Constructor. */ - public function __construct(RequestStack $requestStack, HarvestService $service) { - $this->requestStack = $requestStack; + public function __construct(HarvestService $service) { $this->harvester = $service; } @@ -101,9 +92,9 @@ public function getPlan($identifier) { /** * Register a new harvest. */ - public function register() { + public function register(Request $request) { try { - $harvest_plan = $this->requestStack->getCurrentRequest()->getContent(); + $harvest_plan = $request->getContent(); $plan = json_decode($harvest_plan); $identifier = $this->harvester ->registerHarvest($plan); @@ -146,9 +137,9 @@ public function deregister($identifier) { /** * Runs harvest. */ - public function run() { + public function run(Request $request) { try { - $payloadJson = $this->requestStack->getCurrentRequest()->getContent(); + $payloadJson = $request->getContent(); $payload = json_decode($payloadJson); if (!isset($payload->plan_id)) { $return = [ @@ -179,10 +170,10 @@ public function run() { /** * Gives list of previous runs for a harvest id. */ - public function info() { + public function info(Request $request) { try { - $id = $this->requestStack->getCurrentRequest()->get('plan'); + $id = $request->get('plan'); if (empty($id)) { return $this->missingParameterJsonResponse('plan'); } @@ -210,10 +201,13 @@ public function info() { * * @param string $identifier * The harvest run identifier. + * @param \Symfony\Component\HttpFoundation\Request $request + * The HTTP request we're handling. */ - public function infoRun($identifier) { + public function infoRun($identifier, Request $request) { + + $plan_id = $request->get('plan'); - $plan_id = $this->requestStack->getCurrentRequest()->get('plan'); if (empty($plan_id)) { return $this->missingParameterJsonResponse('plan'); } @@ -236,9 +230,9 @@ public function infoRun($identifier) { /** * Reverts harvest. */ - public function revert() { + public function revert(Request $request) { try { - $plan_id = $this->requestStack->getCurrentRequest()->get('plan'); + $plan_id = $request->get('plan'); if (empty($plan_id)) { return $this->missingParameterJsonResponse('plan'); } diff --git a/modules/harvest/tests/src/Kernel/WebServiceApiTest.php b/modules/harvest/tests/src/Kernel/WebServiceApiTest.php index 9b59678489..c31d8e3cce 100644 --- a/modules/harvest/tests/src/Kernel/WebServiceApiTest.php +++ b/modules/harvest/tests/src/Kernel/WebServiceApiTest.php @@ -193,7 +193,7 @@ public function testDeregister() { */ public function testInfoErrors() { $controller = WebServiceApi::create($this->container); - $response = $controller->info(); + $response = $controller->info(new Request()); $this->assertEquals(400, $response->getStatusCode(), $response->getContent()); $payload = json_decode($response->getContent(), TRUE); $this->assertEquals("Missing 'plan' query parameter value", $payload['message']); @@ -205,10 +205,10 @@ public function testInfoErrors() { 'dkan.harvest.service', $this->getExplodingHarvestService('getRunIdsForHarvest', $message) ); - $this->container->get('request_stack')->push($this->getPlanRequest($plan_id)); + $request = $this->getPlanRequest($plan_id); $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->info()); + $this->assertInstanceOf(Response::class, $response = $controller->info($request)); $this->assertEquals(400, $response->getStatusCode()); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals($message, $payload->message); @@ -229,11 +229,10 @@ public function testInfo() { // @todo Modify the controller so that we pass in a Request object to the // method instead of using the stack. $request = $this->getPlanRequest($plan_identifier); - $this->container->get('request_stack')->push($request); // Get the info before running. This should result in an empty list. $controller = WebServiceApi::create($this->container); - $response = $controller->info(); + $response = $controller->info($request); $this->assertEquals(200, $response->getStatusCode(), $response->getContent()); $this->assertCount(0, json_decode($response->getContent())); @@ -243,8 +242,7 @@ public function testInfo() { $this->assertArrayNotHasKey('errors', $result); // Get info again, now with results. - $this->container->get('request_stack')->push($request); - $response = $controller->info(); + $response = $controller->info($request); $this->assertEquals(200, $response->getStatusCode(), $response->getContent()); $this->assertCount(1, json_decode($response->getContent())); } @@ -254,14 +252,14 @@ public function testInfo() { */ public function testInfoRunErrors() { $controller = WebServiceApi::create($this->container); - $response = $controller->infoRun('no_run'); + $response = $controller->infoRun('no_run', new Request()); $this->assertEquals(400, $response->getStatusCode(), $response->getContent()); $payload = json_decode($response->getContent(), TRUE); $this->assertEquals("Missing 'plan' query parameter value", $payload['message']); // Add non-existent plan to our request. - $this->container->get('request_stack')->push($this->getPlanRequest('no_such_plan')); - $response = $controller->infoRun('no_run'); + $request = $this->getPlanRequest('no_such_plan'); + $response = $controller->infoRun('no_run', $request); $this->assertEquals(200, $response->getStatusCode(), $response->getContent()); $payload = json_decode($response->getContent()); $this->assertIsObject($payload); @@ -276,7 +274,10 @@ public function testInfoRunErrors() { ); $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->infoRun($plan_id)); + $this->assertInstanceOf( + Response::class, + $response = $controller->infoRun($plan_id, $this->getPlanRequest('no_such_plan')) + ); $this->assertEquals(400, $response->getStatusCode()); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals($message, $payload->message); @@ -295,11 +296,10 @@ public function testInfoRun() { // Add plan to our request. $request = $this->getPlanRequest($plan_identifier); - $this->container->get('request_stack')->push($request); // Plan exists but run ID does not. $controller = WebServiceApi::create($this->container); - $response = $controller->infoRun('no_run'); + $response = $controller->infoRun('no_run', $request); $this->assertEquals(200, $response->getStatusCode(), $response->getContent()); $payload = json_decode($response->getContent()); $this->assertIsObject($payload); @@ -311,12 +311,10 @@ public function testInfoRun() { $this->assertArrayNotHasKey('errors', $result); // Get the run ID. Method runHarvest does not return this information. - $this->container->get('request_stack')->push($request); - $run_ids = json_decode($controller->info()->getContent()); + $run_ids = json_decode($controller->info($request)->getContent()); // Call infoRun() with both plan and run IDs. - $this->container->get('request_stack')->push($request); - $response = $controller->infoRun(reset($run_ids)); + $response = $controller->infoRun(reset($run_ids), $request); $this->assertEquals(200, $response->getStatusCode(), $response->getContent()); // Response is an object. $this->assertIsObject(json_decode($response->getContent())); @@ -331,7 +329,7 @@ public function testInfoRun() { public function testRevertErrors() { $controller = WebServiceApi::create($this->container); // Call revert() without supplying a plan as a request parameter. - $response = $controller->revert(); + $response = $controller->revert(new Request()); $this->assertEquals(400, $response->getStatusCode(), $response->getContent()); $payload = json_decode($response->getContent(), TRUE); $this->assertEquals("Missing 'plan' query parameter value", $payload['message']); @@ -343,10 +341,10 @@ public function testRevertErrors() { 'dkan.harvest.service', $this->getExplodingHarvestService('revertHarvest', $message) ); - $this->container->get('request_stack')->push($this->getPlanRequest('our_plan')); + $request = $this->getPlanRequest($plan_id); $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->revert($plan_id)); + $this->assertInstanceOf(Response::class, $response = $controller->revert($request)); $this->assertEquals(400, $response->getStatusCode()); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals($message, $payload->message); @@ -367,9 +365,8 @@ public function testRevert() { $request = $this->getPlanRequest($plan_identifier); // Revert the plan before it's been run. - $this->container->get('request_stack')->push($request); $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->revert()); + $this->assertInstanceOf(Response::class, $response = $controller->revert($request)); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals($plan_identifier, $payload->identifier); $this->assertEquals(0, $payload->result); @@ -379,9 +376,8 @@ public function testRevert() { $this->assertEquals('SUCCESS', $run_status['status']['extract'] ?? 'no success'); // Revert the plan again. - $this->container->get('request_stack')->push($request); $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->revert()); + $this->assertInstanceOf(Response::class, $response = $controller->revert($request)); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals($plan_identifier, $payload->identifier); $this->assertEquals(2, $payload->result); @@ -391,8 +387,9 @@ public function testRevert() { * @covers ::run */ public function testRunErrors() { + // Request with no payload. $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->run()); + $this->assertInstanceOf(Response::class, $response = $controller->run(new Request())); $this->assertEquals(400, $response->getStatusCode()); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals('Invalid payload.', $payload->message); @@ -405,14 +402,14 @@ public function testRunErrors() { 'dkan.harvest.service', $this->getExplodingHarvestService('runHarvest', $message) ); - $this->container->get('request_stack')->push(Request::create( + $request = Request::create( 'https://example.com', 'POST', [], [], [], [], json_encode((object) ['plan_id' => $plan_id]) - )); + ); $controller = WebServiceApi::create($this->container); - $this->assertInstanceOf(Response::class, $response = $controller->run()); + $this->assertInstanceOf(Response::class, $response = $controller->run($request)); $this->assertEquals(400, $response->getStatusCode()); $this->assertIsObject($payload = json_decode($response->getContent())); $this->assertEquals($message, $payload->message); diff --git a/modules/harvest/tests/src/Unit/WebServiceApiTest.php b/modules/harvest/tests/src/Unit/WebServiceApiTest.php index e093e3995c..116eb53ba2 100644 --- a/modules/harvest/tests/src/Unit/WebServiceApiTest.php +++ b/modules/harvest/tests/src/Unit/WebServiceApiTest.php @@ -101,7 +101,7 @@ public function testEmptyIndex() { public function testBadPlan() { $this->request = new Request(); $controller = WebServiceApi::create($this->getContainer()); - $response = $controller->register(); + $response = $controller->register($this->request); $this->assertInstanceOf(JsonResponse::class, $response); $this->assertEquals($response->getContent(), json_encode(["message" => "Harvest plan must be a php object."])); } @@ -126,7 +126,7 @@ public function testRegisterAndIndex() { $this->request = $request; $controller = WebServiceApi::create($this->getContainer()); - $response = $controller->register(); + $response = $controller->register($request); $this->assertInstanceOf(JsonResponse::class, $response); $this->assertEquals($response->getContent(), json_encode(["identifier" => "test"])); @@ -139,7 +139,6 @@ public function testRegisterAndIndex() { */ public function testRun() { $options = (new Options()) - ->add("request_stack", RequestStack::class) ->add("dkan.harvest.service", HarvestService::class) ->index(0); @@ -150,13 +149,15 @@ public function testRun() { $container = (new Chain($this)) ->add(Container::class, "get", $options) - ->add(RequestStack::class, 'getCurrentRequest', Request::class) - ->add(Request::class, 'getContent', json_encode((object) ['plan_id' => 'test'])) ->add(HarvestService::class, "runHarvest", Result::class) ->getMock(); + $request = new Request([], [], [], [], [], [], + json_encode((object) ['plan_id' => 'test']) + ); + $controller = WebServiceApi::create($container); - $response = $controller->run(); + $response = $controller->run($request); $this->assertInstanceOf(JsonResponse::class, $response); }