From 029ccd0a3872e8e0df6afdd55a9a23aadb25741a Mon Sep 17 00:00:00 2001 From: cpenny Date: Fri, 2 Mar 2018 10:39:47 +1300 Subject: [PATCH] Catch ValidationExceptions and return ValidationResult messages. --- src/DataFormatter.php | 21 ++++++- src/DataFormatter/JSONDataFormatter.php | 19 ++++++ src/DataFormatter/XMLDataFormatter.php | 60 ++++++++++++++++++ src/RestfulServer.php | 49 +++++++++++++-- tests/unit/RestfulServerTest.php | 14 +++++ .../RestfulServerTestValidationFailure.php | 63 +++++++++++++++++++ 6 files changed, 220 insertions(+), 6 deletions(-) create mode 100644 tests/unit/Stubs/RestfulServerTestValidationFailure.php diff --git a/src/DataFormatter.php b/src/DataFormatter.php index 2e2ace6..b093a08 100644 --- a/src/DataFormatter.php +++ b/src/DataFormatter.php @@ -260,6 +260,9 @@ abstract class DataFormatter return $this->removeFields; } + /** + * @return string + */ public function getOutputContentType() { return $this->outputContentType; @@ -340,15 +343,29 @@ abstract class DataFormatter abstract public function supportedMimeTypes(); /** - * Convert a single data object to this format. Return a string. + * Convert a single data object to this format. Return a string. + * + * @param DataObjectInterface $do + * @return mixed */ abstract public function convertDataObject(DataObjectInterface $do); /** - * Convert a data object set to this format. Return a string. + * Convert a data object set to this format. Return a string. + * + * @param SS_List $set + * @return string */ abstract public function convertDataObjectSet(SS_List $set); + /** + * Convert an array to this format. Return a string. + * + * @param $array + * @return string + */ + abstract public function convertArray($array); + /** * @param string $strData HTTP Payload as string */ diff --git a/src/DataFormatter/JSONDataFormatter.php b/src/DataFormatter/JSONDataFormatter.php index 173af0a..bcc643b 100644 --- a/src/DataFormatter/JSONDataFormatter.php +++ b/src/DataFormatter/JSONDataFormatter.php @@ -22,6 +22,9 @@ class JSONDataFormatter extends DataFormatter protected $outputContentType = 'application/json'; + /** + * @return array + */ public function supportedExtensions() { return array( @@ -30,6 +33,9 @@ class JSONDataFormatter extends DataFormatter ); } + /** + * @return array + */ public function supportedMimeTypes() { return array( @@ -38,6 +44,15 @@ class JSONDataFormatter extends DataFormatter ); } + /** + * @param $array + * @return string + */ + public function convertArray($array) + { + return Convert::array2json($array); + } + /** * Generate a JSON representation of the given {@link DataObject}. * @@ -163,6 +178,10 @@ class JSONDataFormatter extends DataFormatter return Convert::array2json($serobj); } + /** + * @param string $strData + * @return array|bool|void + */ public function convertStringToArray($strData) { return Convert::json2array($strData); diff --git a/src/DataFormatter/XMLDataFormatter.php b/src/DataFormatter/XMLDataFormatter.php index 3876ea7..f2b9673 100644 --- a/src/DataFormatter/XMLDataFormatter.php +++ b/src/DataFormatter/XMLDataFormatter.php @@ -4,6 +4,7 @@ namespace SilverStripe\RestfulServer\DataFormatter; use SilverStripe\Control\Controller; use SilverStripe\Core\Convert; +use SilverStripe\Dev\Debug; use SilverStripe\RestfulServer\DataFormatter; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; @@ -24,6 +25,9 @@ class XMLDataFormatter extends DataFormatter protected $outputContentType = 'text/xml'; + /** + * @return array + */ public function supportedExtensions() { return array( @@ -31,6 +35,9 @@ class XMLDataFormatter extends DataFormatter ); } + /** + * @return array + */ public function supportedMimeTypes() { return array( @@ -39,6 +46,48 @@ class XMLDataFormatter extends DataFormatter ); } + /** + * @param $array + * @return string + * @throws \Exception + */ + public function convertArray($array) + { + $response = Controller::curr()->getResponse(); + if ($response) { + $response->addHeader("Content-Type", "text/xml"); + } + + return "\n + {$this->convertArrayWithoutHeader($array)}"; + } + + /** + * @param $array + * @return string + * @throws \Exception + */ + public function convertArrayWithoutHeader($array) + { + $xml = ''; + + foreach ($array as $fieldName => $fieldValue) { + if (is_array($fieldValue)) { + if (is_numeric($fieldName)) { + $fieldName = 'Item'; + } + + $xml .= "<{$fieldName}>\n"; + $xml .= $this->convertArrayWithoutHeader($fieldValue); + $xml .= "\n"; + } else { + $xml .= "<$fieldName>$fieldValue\n"; + } + } + + return $xml; + } + /** * Generate an XML representation of the given {@link DataObject}. * @@ -56,6 +105,12 @@ class XMLDataFormatter extends DataFormatter return "\n" . $this->convertDataObjectWithoutHeader($obj, $fields); } + /** + * @param DataObject $obj + * @param null $fields + * @param null $relations + * @return string + */ public function convertDataObjectWithoutHeader(DataObject $obj, $fields = null, $relations = null) { $className = $this->sanitiseClassName(get_class($obj)); @@ -195,6 +250,11 @@ class XMLDataFormatter extends DataFormatter return $xml; } + /** + * @param string $strData + * @return array|void + * @throws \Exception + */ public function convertStringToArray($strData) { return Convert::xml2array($strData); diff --git a/src/RestfulServer.php b/src/RestfulServer.php index a245a03..d5791a3 100644 --- a/src/RestfulServer.php +++ b/src/RestfulServer.php @@ -10,6 +10,8 @@ use SilverStripe\ORM\DataObject; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\ORM\SS_List; +use SilverStripe\ORM\ValidationException; +use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\CMS\Model\SiteTree; @@ -443,8 +445,13 @@ class RestfulServer extends Controller return $this->unsupportedMediaType(); } - /** @var DataObject|string */ - $obj = $this->updateDataObject($obj, $reqFormatter); + try { + /** @var DataObject|string */ + $obj = $this->updateDataObject($obj, $reqFormatter); + } catch (ValidationException $e) { + return $this->validationFailure($responseFormatter, $e->getResult()); + } + if (is_string($obj)) { return $obj; } @@ -515,8 +522,13 @@ class RestfulServer extends Controller $responseFormatter = $this->getResponseDataFormatter($className); - /** @var DataObject|string $obj */ - $obj = $this->updateDataObject($obj, $reqFormatter); + try { + /** @var DataObject|string $obj */ + $obj = $this->updateDataObject($obj, $reqFormatter); + } catch (ValidationException $e) { + return $this->validationFailure($responseFormatter, $e->getResult()); + } + if (is_string($obj)) { return $obj; } @@ -643,6 +655,9 @@ class RestfulServer extends Controller } } + /** + * @return string + */ protected function permissionFailure() { // return a 401 @@ -652,6 +667,9 @@ class RestfulServer extends Controller return "You don't have access to this item through the API."; } + /** + * @return string + */ protected function notFound() { // return a 404 @@ -660,6 +678,9 @@ class RestfulServer extends Controller return "That object wasn't found"; } + /** + * @return string + */ protected function methodNotAllowed() { $this->getResponse()->setStatusCode(405); @@ -667,6 +688,9 @@ class RestfulServer extends Controller return "Method Not Allowed"; } + /** + * @return string + */ protected function unsupportedMediaType() { $this->response->setStatusCode(415); // Unsupported Media Type @@ -674,6 +698,23 @@ class RestfulServer extends Controller return "Unsupported Media Type"; } + /** + * @param ValidationResult $result + * @return mixed + */ + protected function validationFailure(DataFormatter $responseFormatter, ValidationResult $result) + { + $this->getResponse()->setStatusCode(400); + $this->getResponse()->addHeader('Content-Type', $responseFormatter->getOutputContentType()); + + $response = [ + 'type' => ValidationException::class, + 'messages' => $result->getMessages(), + ]; + + return $responseFormatter->convertArray($response); + } + /** * A function to authenticate a user * diff --git a/tests/unit/RestfulServerTest.php b/tests/unit/RestfulServerTest.php index 39463ca..22bf37c 100644 --- a/tests/unit/RestfulServerTest.php +++ b/tests/unit/RestfulServerTest.php @@ -10,6 +10,7 @@ use SilverStripe\RestfulServer\Tests\Stubs\RestfulServerTestAuthorRating; use SilverStripe\Control\Director; use SilverStripe\Core\Convert; use SilverStripe\Control\Controller; +use SilverStripe\RestfulServer\Tests\Stubs\RestfulServerTestValidationFailure; use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\ORM\DataObject; @@ -572,4 +573,17 @@ class RestfulServerTest extends SapphireTest unset($_SERVER['PHP_AUTH_USER']); unset($_SERVER['PHP_AUTH_PW']); } + + public function testValidationErrorWithPOST() + { + $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestValidationFailure::class); + $url = "{$this->baseURI}/api/v1/$urlSafeClassname/"; + $data = [ + 'Content' => 'Test', + ]; + $response = Director::test($url, $data, null, 'POST'); + // Assumption: XML is default output + $responseArr = Convert::xml2array($response->getBody()); + $this->assertEquals('SilverStripe\\ORM\\ValidationException', $responseArr['type']); + } } diff --git a/tests/unit/Stubs/RestfulServerTestValidationFailure.php b/tests/unit/Stubs/RestfulServerTestValidationFailure.php new file mode 100644 index 0000000..32e1871 --- /dev/null +++ b/tests/unit/Stubs/RestfulServerTestValidationFailure.php @@ -0,0 +1,63 @@ + 'Text', + 'Title' => 'Text', + ); + + /** + * @return \SilverStripe\ORM\ValidationResult + */ + public function validate() + { + $result = parent::validate(); + + if (strlen($this->Content) === 0) { + $result->addFieldError('Content', 'Content required'); + } + + if (strlen($this->Title) === 0) { + $result->addFieldError('Title', 'Title required'); + } + + return $result; + } + + public function canView($member = null) + { + return true; + } + + public function canEdit($member = null) + { + return true; + } + + public function canDelete($member = null) + { + return true; + } + + public function canCreate($member = null, $context = array()) + { + return true; + } +}