From 960171c4e09cd5d95253e29f46f3a1a2f585be31 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 22 Sep 2023 10:13:35 -0400 Subject: [PATCH] Remove dependency on super-globals in Module\Api\ApiResponse - Updated DI dependencies to reflect the new parameters - Updated tests to reflect the new parameters --- src/Module/Api/ApiResponse.php | 28 ++++++++--- static/dependencies.config.php | 6 +++ tests/src/Module/Api/ApiResponseTest.php | 59 +++++++++++++++++++++++- 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/src/Module/Api/ApiResponse.php b/src/Module/Api/ApiResponse.php index b5b2a4717c..518bcf8cba 100644 --- a/src/Module/Api/ApiResponse.php +++ b/src/Module/Api/ApiResponse.php @@ -25,6 +25,7 @@ use Friendica\App\Arguments; use Friendica\App\BaseURL; use Friendica\Core\L10n; use Friendica\Module\Response; +use Friendica\Network\HTTPException; use Friendica\Util\Arrays; use Friendica\Util\DateTimeFormat; use Friendica\Util\XML; @@ -46,14 +47,20 @@ class ApiResponse extends Response protected $baseUrl; /** @var TwitterUser */ protected $twitterUser; + /** @var array */ + protected $server; + /** @var string */ + protected $jsonpCallback; - public function __construct(L10n $l10n, Arguments $args, LoggerInterface $logger, BaseURL $baseUrl, TwitterUser $twitterUser) + public function __construct(L10n $l10n, Arguments $args, LoggerInterface $logger, BaseURL $baseUrl, TwitterUser $twitterUser, array $server = [], string $jsonpCallback = '') { $this->l10n = $l10n; $this->args = $args; $this->logger = $logger; $this->baseUrl = $baseUrl; $this->twitterUser = $twitterUser; + $this->server = $server; + $this->jsonpCallback = $jsonpCallback; } /** @@ -63,6 +70,7 @@ class ApiResponse extends Response * @param string $root_element Name of the root element * * @return string The XML data + * @throws \Exception */ public function createXML(array $data, string $root_element): string { @@ -109,6 +117,7 @@ class ApiResponse extends Response * @param int $cid Contact ID of template * * @return array + * @throws HTTPException\InternalServerErrorException */ private function addRSSValues(array $arr, int $cid): array { @@ -141,6 +150,7 @@ class ApiResponse extends Response * @param int $cid ID of the contact for RSS * * @return array|string (string|array) XML data or JSON data + * @throws HTTPException\InternalServerErrorException */ public function formatData(string $root_element, string $type, array $data, int $cid = 0) { @@ -180,7 +190,7 @@ class ApiResponse extends Response } /** - * Exit with error code + * Add formatted error message to response * * @param int $code * @param string $description @@ -188,6 +198,7 @@ class ApiResponse extends Response * @param string|null $format * * @return void + * @throws HTTPException\InternalServerErrorException */ public function error(int $code, string $description, string $message, string $format = null) { @@ -197,19 +208,21 @@ class ApiResponse extends Response 'request' => $this->args->getQueryString() ]; - $this->setHeader(($_SERVER['SERVER_PROTOCOL'] ?? 'HTTP/1.1') . ' ' . $code . ' ' . $description); + $this->setHeader(($this->server['SERVER_PROTOCOL'] ?? 'HTTP/1.1') . ' ' . $code . ' ' . $description); $this->exit('status', ['status' => $error], $format); } /** - * Outputs formatted data according to the data type and then exits the execution. + * Add formatted data according to the data type to the response. * * @param string $root_element * @param array $data An array with a single element containing the returned result * @param string|null $format Output format (xml, json, rss, atom) + * @param int $cid * * @return void + * @throws HTTPException\InternalServerErrorException */ public function exit(string $root_element, array $data, string $format = null, int $cid = 0) { @@ -226,8 +239,8 @@ class ApiResponse extends Response $this->setType(static::TYPE_JSON); if (!empty($return)) { $json = json_encode(end($return)); - if (!empty($_GET['callback'])) { - $json = $_GET['callback'] . '(' . $json . ')'; + if ($this->jsonpCallback) { + $json = $this->jsonpCallback . '(' . $json . ')'; } $return = $json; } @@ -251,6 +264,7 @@ class ApiResponse extends Response * @param array $data * * @return void + * @throws HTTPException\InternalServerErrorException */ public function exitWithJson(array $data) { @@ -273,7 +287,7 @@ class ApiResponse extends Response [ 'method' => $method, 'path' => $path, - 'agent' => $_SERVER['HTTP_USER_AGENT'] ?? '', + 'agent' => $this->server['HTTP_USER_AGENT'] ?? '', 'request' => $request, ]); $error = $this->l10n->t('API endpoint %s %s is not implemented but might be in the future.', strtoupper($method), $path); diff --git a/static/dependencies.config.php b/static/dependencies.config.php index 3dfde6b5f4..830b7fec38 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -330,4 +330,10 @@ return [ $_SERVER ], ], + \Friendica\Module\Api\ApiResponse::class => [ + 'constructParams' => [ + $_SERVER, + $_GET['callback'] ?? '', + ], + ], ]; diff --git a/tests/src/Module/Api/ApiResponseTest.php b/tests/src/Module/Api/ApiResponseTest.php index 2defaf995c..0c246e58a2 100644 --- a/tests/src/Module/Api/ApiResponseTest.php +++ b/tests/src/Module/Api/ApiResponseTest.php @@ -127,7 +127,30 @@ class ApiResponseTest extends MockedTest $baseUrl = \Mockery::mock(BaseURL::class); $twitterUser = \Mockery::mock(User::class); - $response = new ApiResponse($l10n, $args, new NullLogger(), $baseUrl, $twitterUser); + $logger = \Mockery::mock(NullLogger::class); + $logger->shouldReceive('info')->withArgs(['Unimplemented API call', ['method' => 'all', 'path' => '', 'agent' => '', 'request' => []]]); + + $response = new ApiResponse($l10n, $args, $logger, $baseUrl, $twitterUser); + $response->unsupported(); + + self::assertEquals('{"error":"API endpoint %s %s is not implemented but might be in the future.","code":"501 Not Implemented","request":""}', $response->getContent()); + } + + public function testUnsupportedUserAgent() + { + $l10n = \Mockery::mock(L10n::class); + $l10n->shouldReceive('t')->andReturnUsing(function ($args) { + return $args; + }); + $args = \Mockery::mock(Arguments::class); + $args->shouldReceive('getQueryString')->andReturn(''); + $baseUrl = \Mockery::mock(BaseURL::class); + $twitterUser = \Mockery::mock(User::class); + + $logger = \Mockery::mock(NullLogger::class); + $logger->shouldReceive('info')->withArgs(['Unimplemented API call', ['method' => 'all', 'path' => '', 'agent' => 'PHPUnit', 'request' => []]]); + + $response = new ApiResponse($l10n, $args, $logger, $baseUrl, $twitterUser, ['HTTP_USER_AGENT' => 'PHPUnit']); $response->unsupported(); self::assertEquals('{"error":"API endpoint %s %s is not implemented but might be in the future.","code":"501 Not Implemented","request":""}', $response->getContent()); @@ -250,6 +273,40 @@ class ApiResponseTest extends MockedTest self::assertEquals($data, $response->formatData('root_element', 'json', $data)); } + public function testApiExitWithJson() + { + $l10n = \Mockery::mock(L10n::class); + $l10n->shouldReceive('t')->andReturnUsing(function ($args) { + return $args; + }); + $args = \Mockery::mock(Arguments::class); + $args->shouldReceive('getQueryString')->andReturn(''); + $baseUrl = \Mockery::mock(BaseURL::class); + $twitterUser = \Mockery::mock(User::class); + + $response = new ApiResponse($l10n, $args, new NullLogger(), $baseUrl, $twitterUser); + $response->exitWithJson(['some_data']); + + self::assertEquals('["some_data"]', $response->getContent()); + } + + public function testApiExitWithJsonP() + { + $l10n = \Mockery::mock(L10n::class); + $l10n->shouldReceive('t')->andReturnUsing(function ($args) { + return $args; + }); + $args = \Mockery::mock(Arguments::class); + $args->shouldReceive('getQueryString')->andReturn(''); + $baseUrl = \Mockery::mock(BaseURL::class); + $twitterUser = \Mockery::mock(User::class); + + $response = new ApiResponse($l10n, $args, new NullLogger(), $baseUrl, $twitterUser, [], 'JsonPCallback'); + $response->exitWithJson(['some_data']); + + self::assertEquals('JsonPCallback(["some_data"])', $response->getContent()); + } + /** * Test the BaseApi::formatData() function with an XML result. *