From b9b2dc37bb6c15e3c67788d1514a7aa657ed8827 Mon Sep 17 00:00:00 2001 From: Dennis Lassiter Date: Tue, 17 Sep 2024 16:52:33 +0200 Subject: [PATCH] :sparkles: REST: Allow class based output fields When performing core/get on REST Service, you may now define the output_fields parameter based on the class of the object being returned. Previously you could either only return fields from the parent object or enforce return all the fields for each object (with `*+`). With this commit you can also pass `:;:;...`. --- application/applicationextension.inc.php | 84 ++++++++++----- core/restservices.class.inc.php | 7 +- .../unitary-tests/webservices/RestTest.php | 56 +++++++++- .../webservices/RestUtilsTest.php | 101 ++++++++++++++++++ 4 files changed, 218 insertions(+), 30 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/webservices/RestUtilsTest.php diff --git a/application/applicationextension.inc.php b/application/applicationextension.inc.php index 7177c3606b..db10eb3823 100644 --- a/application/applicationextension.inc.php +++ b/application/applicationextension.inc.php @@ -1914,38 +1914,72 @@ public static function GetClass($oData, $sParamName) public static function GetFieldList($sClass, $oData, $sParamName) { $sFields = self::GetOptionalParam($oData, $sParamName, '*'); - $aShowFields = array(); - if ($sFields == '*') - { - foreach (MetaModel::ListAttributeDefs($sClass) as $sAttCode => $oAttDef) - { - $aShowFields[$sClass][] = $sAttCode; - } + return match($sFields) { + '*' => self::GetFieldListForClass($sClass), + '*+' => self::GetFieldListForParentClass($sClass), + default => self::GetLimitedFieldListForClass($sClass, $sFields, $sParamName), + }; + } + + public static function HasRequestedExtendedOutput(string $sFields): bool + { + return match($sFields) { + '*' => false, + '*+' => true, + default => substr_count($sFields, ':') > 1, + }; + } + + public static function HasRequestedAllOutputFields(string $sFields): bool + { + return match($sFields) { + '*', '*+' => true, + default => false, + }; + } + + protected static function GetFieldListForClass(string $sClass): array + { + return [$sClass => array_keys(MetaModel::ListAttributeDefs($sClass))]; + } + + protected static function GetFieldListForParentClass(string $sClass): array + { + $aFieldList = array(); + foreach (MetaModel::EnumChildClasses($sClass, ENUM_CHILD_CLASSES_ALL) as $sRefClass) { + $aFieldList = array_merge($aFieldList, self::GetFieldListForClass($sRefClass)); } - elseif ($sFields == '*+') + return $aFieldList; + } + + protected static function GetLimitedFieldListForSingleClass(string $sClass, string $sFields, string $sParamName): array + { + $aFieldList = [$sClass => []]; + foreach (explode(',', $sFields) as $sAttCode) { - foreach (MetaModel::EnumChildClasses($sClass, ENUM_CHILD_CLASSES_ALL) as $sRefClass) + $sAttCode = trim($sAttCode); + if (($sAttCode != 'id') && (!MetaModel::IsValidAttCode($sClass, $sAttCode))) { - foreach (MetaModel::ListAttributeDefs($sRefClass) as $sAttCode => $oAttDef) - { - $aShowFields[$sRefClass][] = $sAttCode; - } + throw new Exception("$sParamName: invalid attribute code '$sAttCode'"); } + $aFieldList[$sClass][] = $sAttCode; } - else - { - foreach (explode(',', $sFields) as $sAttCode) - { - $sAttCode = trim($sAttCode); - if (($sAttCode != 'id') && (!MetaModel::IsValidAttCode($sClass, $sAttCode))) - { - throw new Exception("$sParamName: invalid attribute code '$sAttCode'"); - } - $aShowFields[$sClass][] = $sAttCode; - } + return $aFieldList; + } + + protected static function GetLimitedFieldListForClass(string $sClass, string $sFields, string $sParamName): array + { + if (!str_contains($sFields, ':')) { + return self::GetLimitedFieldListForSingleClass($sClass, $sFields, $sParamName); } - return $aShowFields; + $aFieldList = []; + $aFieldListParts = explode(';', $sFields); + foreach ($aFieldListParts as $sClassFields) { + list($sSubClass, $sSubClassFields) = explode(':', $sClassFields); + $aFieldList = array_merge($aFieldList, self::GetLimitedFieldListForSingleClass(trim($sSubClass), trim($sSubClassFields), $sParamName)); + } + return $aFieldList; } /** diff --git a/core/restservices.class.inc.php b/core/restservices.class.inc.php index 3ad0ae74e3..c4198bd076 100644 --- a/core/restservices.class.inc.php +++ b/core/restservices.class.inc.php @@ -503,8 +503,8 @@ public function ExecOperation($sVersion, $sVerb, $aParams) case 'core/get': $sClass = RestUtils::GetClass($aParams, 'class'); $key = RestUtils::GetMandatoryParam($aParams, 'key'); + $sShowFields = RestUtils::GetOptionalParam($aParams, 'output_fields', '*'); $aShowFields = RestUtils::GetFieldList($sClass, $aParams, 'output_fields'); - $bExtendedOutput = (RestUtils::GetOptionalParam($aParams, 'output_fields', '*') == '*+'); $iLimit = (int)RestUtils::GetOptionalParam($aParams, 'limit', 0); $iPage = (int)RestUtils::GetOptionalParam($aParams, 'page', 1); @@ -528,7 +528,8 @@ public function ExecOperation($sVersion, $sVerb, $aParams) } else { - if (!$bExtendedOutput && RestUtils::GetOptionalParam($aParams, 'output_fields', '*') != '*') + + if (!RestUtils::HasRequestedAllOutputFields($sShowFields)) { $aFields = $aShowFields[$sClass]; //Id is not a valid attribute to optimize @@ -542,7 +543,7 @@ public function ExecOperation($sVersion, $sVerb, $aParams) while ($oObject = $oObjectSet->Fetch()) { - $oResult->AddObject(0, '', $oObject, $aShowFields, $bExtendedOutput); + $oResult->AddObject(0, '', $oObject, $aShowFields, RestUtils::HasRequestedExtendedOutput($sShowFields)); } $oResult->message = "Found: ".$oObjectSet->Count(); } diff --git a/tests/php-unit-tests/unitary-tests/webservices/RestTest.php b/tests/php-unit-tests/unitary-tests/webservices/RestTest.php index 6b1c6848a3..70e9bd0a8b 100644 --- a/tests/php-unit-tests/unitary-tests/webservices/RestTest.php +++ b/tests/php-unit-tests/unitary-tests/webservices/RestTest.php @@ -142,6 +142,58 @@ public function testCoreApiGet(){ $this->assertJsonStringEqualsJsonString($sExpectedJsonOuput, $sJSONOutput); } + public function testCoreApiGetWithUnionAndDifferentOutputFields(){ + // Create ticket + $description = date('dmY H:i:s'); + $oUserRequest = $this->CreateSampleTicket($description); + $oChange = $this->CreateSampleTicket($description, 'Change'); + $iUserRequestId = $oUserRequest->GetKey(); + $sUserRequestRef = $oUserRequest->Get('ref'); + $iChangeId = $oChange->GetKey(); + $sChangeRef = $oChange->Get('ref'); + + $sJSONOutput = $this->CallCoreRestApi_Internally(<<assertJsonStringEqualsJsonString($sExpectedJsonOuput, $sJSONOutput); + } + public function testCoreApiCreate() { // Create ticket @@ -253,9 +305,9 @@ public function testCoreApiDelete() // //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - private function CreateSampleTicket($description) + private function CreateSampleTicket($description, $sType = 'UserRequest') { - $oTicket = $this->createObject('UserRequest', [ + $oTicket = $this->createObject($sType, [ 'org_id' => $this->getTestOrgId(), "title" => "Houston, got a problem", "description" => $description diff --git a/tests/php-unit-tests/unitary-tests/webservices/RestUtilsTest.php b/tests/php-unit-tests/unitary-tests/webservices/RestUtilsTest.php new file mode 100644 index 0000000000..d62ae3af7c --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/webservices/RestUtilsTest.php @@ -0,0 +1,101 @@ + 'ref,start_date,end_date'], 'output_fields'); + $this->assertSame([Ticket::class => ['ref', 'start_date', 'end_date']], $aList); + } + + public function testGetFieldListForSingleClassWithInvalidFieldNameFails(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('output_fields: invalid attribute code \'something\''); + $aList = RestUtils::GetFieldList(Ticket::class, (object) ['output_fields' => 'ref,something'], 'output_fields'); + $this->assertSame([Ticket::class => ['ref', 'start_date', 'end_date']], $aList); + } + + public function testGetFieldListWithAsteriskOnParentClass(): void + { + $aList = RestUtils::GetFieldList(Ticket::class, (object) ['output_fields' => '*'], 'output_fields'); + $this->assertArrayHasKey(Ticket::class, $aList); + $this->assertContains('operational_status', $aList[Ticket::class]); + $this->assertNotContains('status', $aList[Ticket::class], 'Representation of Class Ticket should not contain status, since it is defined by children'); + } + + public function testGetFieldListWithAsteriskPlusOnParentClass(): void + { + $aList = RestUtils::GetFieldList(Ticket::class, (object) ['output_fields' => '*+'], 'output_fields'); + $this->assertArrayHasKey(Ticket::class, $aList); + $this->assertArrayHasKey(UserRequest::class, $aList); + $this->assertContains('operational_status', $aList[Ticket::class]); + $this->assertContains('status', $aList[UserRequest::class]); + } + + public function testGetFieldListForMultipleClasses(): void + { + $aList = RestUtils::GetFieldList(Ticket::class, (object) ['output_fields' => 'Ticket:ref,start_date,end_date;UserRequest:ref,status'], 'output_fields'); + $this->assertArrayHasKey(Ticket::class, $aList); + $this->assertArrayHasKey(UserRequest::class, $aList); + $this->assertContains('ref', $aList[Ticket::class]); + $this->assertContains('end_date', $aList[Ticket::class]); + $this->assertNotContains('status', $aList[Ticket::class]); + $this->assertContains('status', $aList[UserRequest::class]); + $this->assertNotContains('end_date', $aList[UserRequest::class]); + } + + public function testGetFieldListForMultipleClassesWithInvalidFieldNameFails(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessage('output_fields: invalid attribute code \'something\''); + RestUtils::GetFieldList(Ticket::class, (object) ['output_fields' => 'Ticket:ref;UserRequest:ref,something'], 'output_fields'); + } + + /** + * @dataProvider extendedOutputDataProvider + */ + public function testIsExtendedOutputRequest(bool $bExpected, string $sFields): void + { + $this->assertSame($bExpected, RestUtils::HasRequestedExtendedOutput($sFields)); + } + + /** + * @dataProvider allFieldsOutputDataProvider + */ + public function testIsAllFieldsOutputRequest(bool $bExpected, string $sFields): void + { + $this->assertSame($bExpected, RestUtils::HasRequestedAllOutputFields($sFields)); + } + + public function extendedOutputDataProvider(): array + { + return [ + [false, 'ref,start_date,end_date'], + [false, '*'], + [true, '*+'], + [false, 'Ticket:ref'], + [true, 'Ticket:ref;UserRequest:ref'], + ]; + } + + public function allFieldsOutputDataProvider(): array + { + return [ + [false, 'ref,start_date,end_date'], + [true, '*'], + [true, '*+'], + [false, 'Ticket:ref'], + [false, 'Ticket:ref;UserRequest:ref'], + ]; + } +}