Skip to content

Commit

Permalink
Use Request in harvest WebServiceApi (#4235)
Browse files Browse the repository at this point in the history
  • Loading branch information
paul-m authored Jul 24, 2024
1 parent 7ccd3c6 commit f51b5fb
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 55 deletions.
36 changes: 15 additions & 21 deletions modules/harvest/src/WebServiceApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*
Expand All @@ -36,16 +29,14 @@ class WebServiceApi implements ContainerInjectionInterface {
*/
public static function create(ContainerInterface $container) {
return new static(
$container->get('request_stack'),
$container->get('dkan.harvest.service')
);
}

/**
* Constructor.
*/
public function __construct(RequestStack $requestStack, HarvestService $service) {
$this->requestStack = $requestStack;
public function __construct(HarvestService $service) {
$this->harvester = $service;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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');
}
Expand All @@ -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');
}
Expand Down
53 changes: 25 additions & 28 deletions modules/harvest/tests/src/Kernel/WebServiceApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand All @@ -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);
Expand All @@ -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()));

Expand All @@ -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()));
}
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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()));
Expand All @@ -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']);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
13 changes: 7 additions & 6 deletions modules/harvest/tests/src/Unit/WebServiceApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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."]));
}
Expand All @@ -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"]));

Expand All @@ -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);

Expand All @@ -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);
}

Expand Down

0 comments on commit f51b5fb

Please sign in to comment.