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'], + ]; + } +}