Fixing issue 109 - Allow requests with empty body #114
Conversation
1 similar comment
2 similar comments
2 similar comments
|
|
||
| } catch(EmptyBodyException $e) { | ||
| throw new EmptyBodyException($e->getMessage()); | ||
| }catch (\Exception $exception) { |
There was a problem hiding this comment.
Could you fix the spacing here and on line 139?
src/Method.php
Outdated
| @@ -13,7 +15,7 @@ class Method implements ArrayInstantiationInterface, MessageSchemaInterface | |||
| * - Currently missing OPTIONS as this is unlikely to be specified in RAML | |||
| * @var array | |||
| */ | |||
| public static $validMethods = ['GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'PATCH']; | |||
| public static $validMethods = ['GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'PATCH', 'OPTIONS']; | |||
There was a problem hiding this comment.
Could you add a unit test with OPTIONS as method?
src/Exception/EmptyBodyException.php
Outdated
| <?php | ||
| namespace Raml\Exception; | ||
|
|
||
|
|
There was a problem hiding this comment.
Please remove the extra new line here.
src/Method.php
Outdated
| @@ -381,8 +383,12 @@ public function addQueryParameter(NamedParameter $queryParameter) | |||
| */ | |||
| public function getBodyByType($type) | |||
| { | |||
| if(empty($this->getBodies())){ | |||
There was a problem hiding this comment.
Please apply the correct spacing style.
src/Validator/RequestValidator.php
Outdated
| $schemaBody->getSchema()->validate($body); | ||
|
|
||
| }catch (EmptyBodyException $exception) { |
There was a problem hiding this comment.
The spacing style is slightly off in this newly added line.
|
Thanks for the review, I will be making the corrections soon. |
|
@jefferson-lima would you be able to address the code review changes? |
| } catch (Exception $exception) { | ||
|
|
||
| } catch(EmptyBodyException $e) { | ||
| throw new EmptyBodyException($e->getMessage()); |
There was a problem hiding this comment.
It doesn't look right that the exception is throwing a new instance of the same exception here (EmptyBodyException).
There was a problem hiding this comment.
I believe that what's not quite right is catching an Exception in such a low level of the code, instead of catching a more specific exception. In this case, I need the EmptyBodyException to be propagated, but if it weren't for that piece of code, it would be caught and rethrown as a ValidatorSchemaException. I understand that this is a workaround and it's not straightforward.
|
Hi @martin-georgiev, I believe I fixed the issues you pointed out. Would you please give it another look? Note that I squashed my previous commits into a single one to don't mess up the history, I hope this is not a problem. |
Here's a fix for the problem described in the issue 109.