From c028755adf946ed4d3678838807eaec484306db9 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Tue, 17 Oct 2023 09:14:46 +1300 Subject: [PATCH] MNT Remove TODO comments --- src/DataFormatter.php | 7 ----- .../FormEncodedDataFormatter.php | 3 -- src/DataFormatter/JSONDataFormatter.php | 1 - src/DataFormatter/XMLDataFormatter.php | 1 - src/RestfulServer.php | 28 ------------------- tests/unit/JSONDataFormatterTest.php | 7 ----- tests/unit/RestfulServerTest.php | 10 ------- 7 files changed, 57 deletions(-) diff --git a/src/DataFormatter.php b/src/DataFormatter.php index 97d6bdb..79ef2a3 100644 --- a/src/DataFormatter.php +++ b/src/DataFormatter.php @@ -32,8 +32,6 @@ abstract class DataFormatter * ($has_one, $has_many, $many_many). * Set to "0" to disable relation output. * - * @todo Support more than one nesting level - * * @var int */ public $relationDepth = 1; @@ -290,9 +288,6 @@ abstract class DataFormatter * Returns all fields on the object which should be shown * in the output. Can be customised through {@link self::setCustomFields()}. * - * @todo Allow for custom getters on the processed object (currently filtered through inheritedDatabaseFields) - * @todo Field level permission checks - * * @param DataObject $obj * @return array */ @@ -303,7 +298,6 @@ abstract class DataFormatter // if custom fields are specified, only select these if (is_array($this->customFields)) { foreach ($this->customFields as $fieldName) { - // @todo Possible security risk by making methods accessible - implement field-level security if (($obj->hasField($fieldName) && !is_object($obj->getField($fieldName))) || $obj->hasMethod("get{$fieldName}") ) { @@ -318,7 +312,6 @@ abstract class DataFormatter if (is_array($this->customAddFields)) { foreach ($this->customAddFields as $fieldName) { - // @todo Possible security risk by making methods accessible - implement field-level security if ($obj->hasField($fieldName) || $obj->hasMethod("get{$fieldName}")) { $dbFields[$fieldName] = $fieldName; } diff --git a/src/DataFormatter/FormEncodedDataFormatter.php b/src/DataFormatter/FormEncodedDataFormatter.php index b136f47..ea5f31b 100644 --- a/src/DataFormatter/FormEncodedDataFormatter.php +++ b/src/DataFormatter/FormEncodedDataFormatter.php @@ -12,7 +12,6 @@ namespace SilverStripe\RestfulServer\DataFormatter; * curl -X PUT -d "Name=This is an updated record" http://host/api/v1/(DataObject)/1 * * - * @todo Format response form encoded as well - currently uses XMLDataFormatter * * @author Cam Spiers */ @@ -37,7 +36,5 @@ class FormEncodedDataFormatter extends XMLDataFormatter $postArray = array(); parse_str($strData ?? '', $postArray); return $postArray; - //TODO: It would be nice to implement this function in Convert.php - //return Convert::querystr2array($strData); } } diff --git a/src/DataFormatter/JSONDataFormatter.php b/src/DataFormatter/JSONDataFormatter.php index a913c32..6f846f0 100644 --- a/src/DataFormatter/JSONDataFormatter.php +++ b/src/DataFormatter/JSONDataFormatter.php @@ -18,7 +18,6 @@ class JSONDataFormatter extends DataFormatter { /** * @config - * @todo pass this from the API to the data formatter somehow */ private static $api_base = "api/v1/"; diff --git a/src/DataFormatter/XMLDataFormatter.php b/src/DataFormatter/XMLDataFormatter.php index 45aa669..58737b0 100644 --- a/src/DataFormatter/XMLDataFormatter.php +++ b/src/DataFormatter/XMLDataFormatter.php @@ -22,7 +22,6 @@ class XMLDataFormatter extends DataFormatter /** * @config - * @todo pass this from the API to the data formatter somehow */ private static $api_base = "api/v1/"; diff --git a/src/RestfulServer.php b/src/RestfulServer.php index 1a14b55..977c3e2 100644 --- a/src/RestfulServer.php +++ b/src/RestfulServer.php @@ -22,23 +22,6 @@ use SilverStripe\Security\Security; * Relies on serialization/deserialization into different formats provided * by the DataFormatter APIs in core. * - * @todo Implement PUT/POST/DELETE for relations - * @todo Access-Control for relations (you might be allowed to view Members and Groups, - * but not their relation with each other) - * @todo Make SearchContext specification customizeable for each class - * @todo Allow for range-searches (e.g. on Created column) - * @todo Filter relation listings by $api_access and canView() permissions - * @todo Exclude relations when "fields" are specified through URL (they should be explicitly - * requested in this case) - * @todo Custom filters per DataObject subclass, e.g. to disallow showing unpublished pages in - * SiteTree/Versioned/Hierarchy - * @todo URL parameter namespacing for search-fields, limit, fields, add_fields - * (might all be valid dataobject properties) - * e.g. you wouldn't be able to search for a "limit" property on your subclass as - * its overlayed with the search logic - * @todo i18n integration (e.g. Page/1.xml?lang=de_DE) - * @todo Access to extendable methods/relations like SiteTree/1/Versions or SiteTree/1/Version/22 - * @todo Respect $api_access array notation in search contexts */ class RestfulServer extends Controller { @@ -119,7 +102,6 @@ class RestfulServer extends Controller { /* This sets up SiteTree the same as when viewing a page through the frontend. Versioned defaults * to Stage, and then when viewing the front-end Versioned::choose_site_stage changes it to Live. - * TODO: In 3.2 we should make the default Live, then change to Stage in the admin area (with a nicer API) */ if (class_exists(SiteTree::class)) { singleton(SiteTree::class)->extend('modelascontrollerInit', $this); @@ -262,8 +244,6 @@ class RestfulServer extends Controller * - static $api_access must be set. This enables the API on a class by class basis * - $obj->canView() must return true. This lets you implement record-level security * - * @todo Access checking - * * @param string $className * @param int $id * @param string $relation @@ -319,7 +299,6 @@ class RestfulServer extends Controller return $this->notFound(); } - // TODO Avoid creating data formatter again for relation class (see above) $responseFormatter = $this->getResponseDataFormatter($obj->dataClass()); } } else { @@ -362,8 +341,6 @@ class RestfulServer extends Controller * an existing query object (mostly a component query from {@link DataObject}) * with search clauses. * - * @todo Allow specifying of different searchcontext getters on model-by-model basis - * * @param string $className * @param array $params * @return SS_List @@ -556,9 +533,6 @@ class RestfulServer extends Controller /** * Handler for object append / method call. * - * @todo Posting to an existing URL (without a relation) - * current resolves in creatig a new element, - * rather than a "Conflict" message. */ protected function postHandler($className, $id, $relation) { @@ -678,7 +652,6 @@ class RestfulServer extends Controller $data[$newkey] = $value; } - // @todo Disallow editing of certain keys in database $data = array_diff_key($data ?? [], ['ID', 'Created']); $apiAccess = singleton($className)->config()->api_access; @@ -866,7 +839,6 @@ class RestfulServer extends Controller /** * Return only relations which have $api_access enabled. - * @todo Respect field level permissions once they are available in core * * @param string $class * @param Member $member diff --git a/tests/unit/JSONDataFormatterTest.php b/tests/unit/JSONDataFormatterTest.php index 86ed7c1..3feb3a0 100644 --- a/tests/unit/JSONDataFormatterTest.php +++ b/tests/unit/JSONDataFormatterTest.php @@ -7,13 +7,6 @@ use SilverStripe\RestfulServer\Tests\Stubs\JSONDataFormatterTypeTestObject; use SilverStripe\Dev\SapphireTest; use SilverStripe\RestfulServer\DataFormatter\JSONDataFormatter; -/** - * - * @todo Test Relation getters - * @todo Test filter and limit through GET params - * @todo Test DELETE verb - * - */ class JSONDataFormatterTest extends SapphireTest { protected static $fixture_file = 'JSONDataFormatterTest.yml'; diff --git a/tests/unit/RestfulServerTest.php b/tests/unit/RestfulServerTest.php index 654ee8d..154c66d 100644 --- a/tests/unit/RestfulServerTest.php +++ b/tests/unit/RestfulServerTest.php @@ -22,13 +22,6 @@ use Page; use SilverStripe\Core\Config\Config; use SilverStripe\RestfulServer\DataFormatter\XMLDataFormatter; -/** - * - * @todo Test Relation getters - * @todo Test filter and limit through GET params - * @todo Test DELETE verb - * - */ class RestfulServerTest extends SapphireTest { protected static $fixture_file = 'RestfulServerTest.yml'; @@ -101,7 +94,6 @@ class RestfulServerTest extends SapphireTest $thing1 = $this->objFromFixture(RestfulServerTestSecretThing::class, 'thing1'); $comment1 = $this->objFromFixture(RestfulServerTestComment::class, 'comment1'); - // @todo create additional mock object with authenticated VIEW permissions $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestSecretThing::class); $url = "{$this->baseURI}/api/v1/$urlSafeClassname/" . $thing1->ID; $response = Director::test($url, null, null, 'GET'); @@ -158,7 +150,6 @@ class RestfulServerTest extends SapphireTest $rating1 = $this->objFromFixture(RestfulServerTestAuthorRating::class, 'rating1'); $rating2 = $this->objFromFixture(RestfulServerTestAuthorRating::class, 'rating2'); - // @todo should be set up by fixtures, doesn't work for some reason... $author1->Ratings()->add($rating1); $author1->Ratings()->add($rating2); @@ -187,7 +178,6 @@ class RestfulServerTest extends SapphireTest $author1 = $this->objFromFixture(RestfulServerTestAuthor::class, 'author1'); $rating1 = $this->objFromFixture(RestfulServerTestAuthorRating::class, 'rating1'); - // @todo should be set up by fixtures, doesn't work for some reason... $author1->Ratings()->add($rating1); $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthor::class);