Skip to content

Commit

Permalink
ENH Standardise API responses
Browse files Browse the repository at this point in the history
  • Loading branch information
@emteknetnz
emteknetnz committed Jan 23, 2024
1 parent 4bc4883 commit a7bd1b0
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 57 deletions.
12 changes: 1 addition & 11 deletions lang/en.yml
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,7 @@
en:
SilverStripe\LinkField\Controllers\LinkFieldController:
BAD_DATA: 'Bad data'
CREATE_LINK: 'Create link'
EMPTY_DATA: 'Empty data'
INVALID_ID: 'Invalid ID'
INVALID_OWNER: 'Invalid Owner'
INVALID_OWNER_CLASS: 'Invalid ownerClass'
INVALID_OWNER_ID: 'Invalid ownerID'
INVALID_OWNER_RELATION: 'Invalid ownerRelation'
INVALID_TOKEN: 'Invalid CSRF token'
INVALID_TYPEKEY: 'Invalid typeKey'
MENUTITLE: SilverStripe\LinkField\Controllers\LinkFieldController
UNAUTHORIZED: Unauthorized
MENUTITLE: 'Link fields'
UPDATE_LINK: 'Update link'
SilverStripe\LinkField\Form\Traits\AllowedLinkClassesTrait:
INVALID_TYPECLASS: '"{class}": {typeclass} is not a valid Link Type'
Expand Down
73 changes: 31 additions & 42 deletions src/Controllers/LinkFieldController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,17 @@ public function linkForm(): Form
if ($id) {
$link = Link::get()->byID($id);
if (!$link) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
$operation = 'edit';
if (!$link->canView()) {
$this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
$this->jsonError(403, 'Unauthorized');
}
} else {
$typeKey = $this->typeKeyFromRequest();
$link = LinkTypeService::create()->byKey($typeKey);
if (!$link) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_TYPEKEY', 'Invalid typeKey'));
$this->jsonError(404, 'Invalid typeKey');
}
$operation = 'create';
}
Expand All @@ -105,17 +105,13 @@ public function linkData(HTTPRequest $request): HTTPResponse
$data[$link->ID] = $this->getLinkData($link);
}
}

$response = $this->getResponse();
$response->addHeader('Content-type', 'application/json');
$response->setBody(json_encode($data));
return $response;
return $this->jsonSuccess(200, $data);
}

private function getLinkData(Link $link): array
{
if (!$link->canView()) {
$this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
$this->jsonError(403, 'Unauthorized');
}
$data = $link->jsonSerialize();
$data['canDelete'] = $link->canDelete();
Expand All @@ -132,11 +128,11 @@ public function linkDelete(): HTTPResponse
{
$link = $this->linkFromRequest();
if (!$link->canDelete()) {
$this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
$this->jsonError(403, 'Unauthorized');
}
// Check security token on destructive operation
if (!SecurityToken::inst()->checkRequest($this->getRequest())) {
$this->jsonError(400, _t(__CLASS__ . '.INVALID_TOKEN', 'Invalid CSRF token'));
$this->jsonError(400, 'Invalid CSRF token');
}
// delete() will also delete any published version immediately
$link->delete();
Expand All @@ -149,10 +145,7 @@ public function linkDelete(): HTTPResponse
$owner->write();
}
// Send response
$response = $this->getResponse();
$response->addHeader('Content-type', 'application/json');
$response->setBody(json_encode(['success' => true]));
return $response;
return $this->jsonSuccess(201);
}

/**
Expand All @@ -172,32 +165,31 @@ public function getLinkForm(): Form
public function save(array $data, Form $form): HTTPResponse
{
if (empty($data)) {
$this->jsonError(400, _t(__CLASS__ . '.EMPTY_DATA', 'Empty data'));
$this->jsonError(400, 'Empty data');
}

/** @var Link $link */
$id = $this->itemIDFromRequest();
if ($id) {
// Editing an existing Link
$operation = 'edit';
$link = Link::get()->byID($id);
if (!$link) {
$this->jsonErorr(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonErorr(404, 'Invalid ID');
}
if (!$link->canEdit()) {
$this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
$this->jsonError(403, 'Unauthorized');
}
} else {
// Creating a new Link
$operation = 'create';
$typeKey = $this->typeKeyFromRequest();
$className = LinkTypeService::create()->byKey($typeKey) ?? '';
if (!$className) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_TYPEKEY', 'Invalid typeKey'));
$this->jsonError(404, 'Invalid typeKey');
}
$link = $className::create();
if (!$link->canCreate()) {
$this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
$this->jsonError(403, 'Unauthorized');
}
}

Expand All @@ -208,7 +200,7 @@ public function save(array $data, Form $form): HTTPResponse
if ((isset($data['ID']) && ((int) $data['ID'] !== $id))
|| isset($data['Sort'])
) {
$this->jsonError(400, _t(__CLASS__ . '.BAD_DATA', 'Bad data'));
$this->jsonError(400, 'Bad data');
}

// Update DataObject from form data
Expand Down Expand Up @@ -272,13 +264,13 @@ public function linkSort()
$request = $this->getRequest();
// Check security token
if (!SecurityToken::inst()->checkRequest($request)) {
$this->jsonError(400, _t(__CLASS__ . '.INVALID_TOKEN', 'Invalid CSRF token'));
$this->jsonError(400, 'Invalid CSRF token');
}
$json = json_decode($request->getBody() ?? '');
$newLinkIDs = $json?->newLinkIDs;
// If someone's passing a JSON object or other non-array here, they're doing something wrong
if (!is_array($newLinkIDs) || empty($newLinkIDs)) {
$this->jsonError(400, _t('LinkField.BAD_DATA', 'Bad data'));
$this->jsonError(400, 'Bad data');
}
// Fetch and validate links
$links = Link::get()->filter(['ID' => $newLinkIDs])->toArray();
Expand All @@ -293,7 +285,7 @@ public function linkSort()
$ownerRelation = $link->OwnerRelation;
}
if ($link->OwnerID !== $ownerID || $link->OwnerRelation !== $ownerRelation) {
$this->jsonError(400, _t('LinkField.BAD_DATA', 'Bad data'));
$this->jsonError(400, 'Bad data');
}
$linkIDToLink[$link->ID] = $link;
}
Expand All @@ -305,7 +297,7 @@ public function linkSort()
// There's also corresponding logic in Link::onBeforeWrite() to also have a minimum of 1
$sort = $i + 1;
if ($link->Sort !== $sort && !$link->canEdit()) {
$this->jsonError(403, _t(__CLASS__ . '.UNAUTHORIZED', 'Unauthorized'));
$this->jsonError(403, 'Unauthorized');
}
}
// Update Sort field on links
Expand All @@ -319,10 +311,7 @@ public function linkSort()
}
}
// Send response
$response = $this->getResponse();
$response->addHeader('Content-type', 'application/json');
$response->setBody(json_encode(['success' => true]));
return $response;
return $this->jsonSuccess(201);
}

/**
Expand Down Expand Up @@ -394,11 +383,11 @@ private function linkFromRequest(): Link
{
$itemID = $this->itemIDFromRequest();
if (!$itemID) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
$link = Link::get()->byID($itemID);
if (!$link) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
return $link;
}
Expand All @@ -410,11 +399,11 @@ private function linksFromRequest(): DataList
{
$itemIDs = $this->itemIDsFromRequest();
if (empty($itemIDs)) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
$links = Link::get()->byIDs($itemIDs);
if (!$links->exists()) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
return $links;
}
Expand All @@ -427,7 +416,7 @@ private function itemIDFromRequest(): int
$request = $this->getRequest();
$itemID = (string) $request->param('ItemID');
if (!ctype_digit($itemID)) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
return (int) $itemID;
}
Expand All @@ -441,13 +430,13 @@ private function itemIDsFromRequest(): array
$itemIDs = $request->getVar('itemIDs');

if (!is_array($itemIDs)) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}

$idsAsInt = [];
foreach ($itemIDs as $id) {
if (!is_int($id) && !ctype_digit($id)) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_ID', 'Invalid ID'));
$this->jsonError(404, 'Invalid ID');
}
$idsAsInt[] = (int) $id;
}
Expand All @@ -463,7 +452,7 @@ private function typeKeyFromRequest(): string
$request = $this->getRequest();
$typeKey = (string) $request->getVar('typeKey');
if (strlen($typeKey) === 0 || !preg_match('#^[a-z\-]+$#', $typeKey)) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_TYPEKEY', 'Invalid typeKey'));
$this->jsonError(404, 'Invalid typeKey');
}
return $typeKey;
}
Expand All @@ -477,11 +466,11 @@ private function ownerFromRequest(): DataObject
$request = $this->getRequest();
$ownerID = (int) ($request->getVar('ownerID') ?: $request->postVar('OwnerID'));
if ($ownerID === 0) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER_ID', 'Invalid ownerID'));
$this->jsonError(404, 'Invalid ownerID');
}
$ownerClass = $request->getVar('ownerClass') ?: $request->postVar('OwnerClass');
if (!is_a($ownerClass, DataObject::class, true)) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER_CLASS', 'Invalid ownerClass'));
$this->jsonError(404, 'Invalid ownerClass');
}
$ownerRelation = $this->ownerRelationFromRequest();
/** @var DataObject $obj */
Expand All @@ -506,7 +495,7 @@ private function ownerFromRequest(): DataObject
return $owner;
}
}
$this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER', 'Invalid Owner'));
$this->jsonError(404, 'Invalid Owner');
}

/**
Expand All @@ -518,7 +507,7 @@ private function ownerRelationFromRequest(): string
$request = $this->getRequest();
$ownerRelation = $request->getVar('ownerRelation') ?: $request->postVar('OwnerRelation');
if (!$ownerRelation) {
$this->jsonError(404, _t(__CLASS__ . '.INVALID_OWNER_RELATION', 'Invalid ownerRelation'));
$this->jsonError(404, 'Invalid ownerRelation');
}
return $ownerRelation;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/php/Controllers/LinkFieldControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ public function testLinkDelete(
$response = $this->mainSession->sendRequest('DELETE', $url, [], $headers);
$this->assertSame('application/json', $response->getHeader('Content-type'));
$this->assertSame($expectedCode, $response->getStatusCode());
if ($expectedCode !== 200) {
if ($expectedCode >= 400) {
$jsonError = json_decode($response->getBody(), true);
$this->assertSame($expectedMessage, $jsonError['errors'][0]['value']);
$this->assertNotNull(TestPhoneLink::get()->byID($fixtureID));
Expand All @@ -549,7 +549,7 @@ public function provideLinkDelete(): array
'Valid' => [
'idType' => 'existing',
'fail' => '',
'expectedCode' => 200,
'expectedCode' => 201,
'expectedMessage' => '',
],
'Reject fail canDelete()' => [
Expand Down Expand Up @@ -622,7 +622,7 @@ public function testLinkSort(
}
$response = $this->post($url, null, $headers, null, $body);
$this->assertSame($expectedCode, $response->getStatusCode());
if ($expectedCode !== 200) {
if ($expectedCode >= 400) {
$jsonError = json_decode($response->getBody(), true);
$this->assertSame($expectedMessage, $jsonError['errors'][0]['value']);
}
Expand All @@ -638,7 +638,7 @@ public function provideLinkSort(): array
'Success' => [
'newTitleOrder' => [4, 2, 3],
'fail' => '',
'expectedCode' => 200,
'expectedCode' => 201,
'expectedMessage' => '',
'expectedTitles' => [4, 2, 3],
],
Expand Down

0 comments on commit a7bd1b0

Please sign in to comment.