-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Naming params added custom response code #3
base: naming_params
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
phpunit.xml | ||
vendor | ||
composer.lock | ||
composer.lock | ||
.idea/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,17 +48,17 @@ public function createMethodCall(Request $request) | |
{ | ||
$content = $request->getContent(); | ||
if (empty($content)) { | ||
throw new InvalidJsonRpcContent('The JSON-RPC request is empty', self::ERROR_INVALID_REQUEST); | ||
throw new InvalidJsonRpcContent('The JSON-RPC request is empty', self::ERROR_INVALID_REQUEST, self::BAD_REQUEST_STATUS_CODE); | ||
} | ||
|
||
$data = json_decode($content, true); | ||
|
||
if (is_null($data)) { | ||
throw new InvalidJsonRpcContent('The JSON-RPC call is not valid', self::ERROR_PARSING); | ||
throw new InvalidJsonRpcContent('The JSON-RPC call is not valid', self::ERROR_PARSING, self::ERROR_STATUS_CODE); | ||
} | ||
|
||
if (empty($data['jsonrpc']) || version_compare($data['jsonrpc'], '2.0', '<')) { | ||
throw new InvalidJsonRpcVersion('The JSON-RPC call version is not supported', self::ERROR_SERVER_ERROR); | ||
throw new InvalidJsonRpcVersion('The JSON-RPC call version is not supported', self::ERROR_SERVER_ERROR, self::ERROR_INVALID_REQUEST); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
if (empty($data['method']) || !isset($data['params'])) { | ||
|
@@ -71,11 +71,12 @@ public function createMethodCall(Request $request) | |
/** | ||
* @param MethodResponse $response | ||
* | ||
* @param int $statusCode | ||
* @return Response | ||
* | ||
* @throws \Seven\RpcBundle\Exception\UnknownMethodResponse | ||
* @throws UnknownMethodResponse | ||
*/ | ||
public function createHttpResponse(MethodResponse $response) | ||
public function createHttpResponse(MethodResponse $response, $statusCode = Server::HTTP_SUCCESS_STATUS) | ||
{ | ||
$data = array( | ||
'jsonrpc' => '2.0', | ||
|
@@ -116,7 +117,7 @@ public function createHttpResponse(MethodResponse $response) | |
$data['id'] = $response->getCallId(); | ||
} | ||
|
||
return new Response(json_encode($data), 200, array('content-type' => 'application/json')); | ||
return new Response(json_encode($data), $statusCode, array('content-type' => 'application/json')); | ||
} | ||
|
||
/** | ||
|
@@ -155,7 +156,7 @@ public function createMethodResponse(Response $response) | |
throw new InvalidJsonRpcContent('The JSON-RPC fault code is not passed'); | ||
} | ||
|
||
$exception = new Fault($data['error']['message'], $data['error']['code']); | ||
$exception = new Fault($data['error']['message'], $data['error']['code'], $response->getStatusCode()); | ||
if (isset($data['error']['data'])) { | ||
$exception->setData($data['error']['data']); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,10 @@ | |
abstract class Implementation | ||
{ | ||
|
||
const ERROR_STATUS_CODE = 500; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Symfony Response has its own statuc code constants: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Response.php#L21-L82 since Symfony 2.4 There is no reason to define same own constants. |
||
const BAD_REQUEST_STATUS_CODE = 400; | ||
const NON_EXIST_STATUS_CODE = 404; | ||
|
||
/** | ||
* @param Request $request | ||
* @return mixed | ||
|
@@ -28,18 +32,19 @@ abstract class Implementation | |
abstract public function createMethodCall(Request $request); | ||
|
||
/** | ||
* @param Response $response | ||
* @param Response $response | ||
* @return MethodResponse | ||
*/ | ||
|
||
abstract public function createMethodResponse(Response $response); | ||
|
||
/** | ||
* @param MethodResponse $response | ||
* @param int $statusCode | ||
* @return Response | ||
*/ | ||
|
||
abstract public function createHttpResponse(MethodResponse $response); | ||
abstract public function createHttpResponse(MethodResponse $response, $statusCode = Server::HTTP_SUCCESS_STATUS); | ||
|
||
/** | ||
* @param MethodCall $call | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,8 @@ | |
|
||
class Server implements ServerInterface | ||
{ | ||
const HTTP_SUCCESS_STATUS = 200; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use Symfony's response constants and remove this. |
||
|
||
protected $impl; | ||
protected $handlers; | ||
|
||
|
@@ -61,6 +63,7 @@ public function handle(Request $request) | |
try { | ||
$methodCall = $this->impl->createMethodCall($request); | ||
$methodResponse = $this->_handle($methodCall); | ||
$exceptionStatus = self::HTTP_SUCCESS_STATUS; | ||
} catch (\Exception $e) { | ||
|
||
// log exception | ||
|
@@ -69,15 +72,17 @@ public function handle(Request $request) | |
} | ||
|
||
$methodResponse = new MethodFault($e); | ||
$exceptionStatus = method_exists($e, 'getHttpStatusCode') ? $e->getHttpStatusCode() : Implementation::BAD_REQUEST_STATUS_CODE; | ||
} | ||
|
||
return $this->impl->createHttpResponse($methodResponse); | ||
return $this->impl->createHttpResponse($methodResponse, $exceptionStatus); | ||
} | ||
|
||
/** | ||
* @param MethodCall $methodCall | ||
* | ||
* @return MethodResponse | ||
* @throws MethodNotExists | ||
* @throws InvalidParameters | ||
*/ | ||
protected function _handle(MethodCall $methodCall) | ||
{ | ||
|
@@ -97,6 +102,7 @@ protected function _handle(MethodCall $methodCall) | |
* @throws MethodNotExists | ||
* | ||
* @return mixed | ||
* @throws InvalidParameters | ||
*/ | ||
public function call($method, $parameters) | ||
{ | ||
|
@@ -129,7 +135,7 @@ protected function isAssociative(array $arr) | |
* Prepare parameters. | ||
* | ||
* @param callable $callback | ||
* @param array $parameters | ||
* @param array $parameters | ||
* | ||
* @return array | ||
* | ||
|
@@ -151,7 +157,7 @@ protected function prepareParameters($callback, array $parameters) | |
) { | ||
throw new InvalidParameters( | ||
sprintf('Invalid number of parameters. %d given but %d are required of %d total.', | ||
$paramCount, $refl->getNumberOfRequiredParameters(), $refl->getNumberOfParameters()) | ||
$paramCount, $refl->getNumberOfRequiredParameters(), $refl->getNumberOfParameters()) | ||
); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
__construct
must always be the first method to increase readability according to https://symfony.com/doc/current/contributing/code/standards.html