N°7914 :sparkles: REST: Allow class based output fields

[delassiter]

Base information

Question	Answer
Related to a SourceForge thead / Another PR / Combodo ticket?	N/A
Type of change?	Enhancement

Objective

When performing core/get with multiple return types (parent and child classes) on REST Service, you may either only return fields from the parent object or enforce return all the fields for each object (with *+).

When, for example, requesting information for Tickets of various types and also including some, but not all the custom properties in the output, you can not provide output_fields which may exist in all implementations, if it does not exist in the requested parent implementation.

The following is currently not possible:

{
   "operation": "core/get",
   "class": "Ticket",
   "key": "SELECT UserRequest UNION SELECT Change",
   "output_fields": "ref,status,finalclass"
}

This is because the "status" is not defined on "Ticket", but rather in each implementation. Querying with "*+" is possible, but then you also request, join, process and transfer a lot of data that was not required.

Proposed solution

My proposed change makes it possible to optionally define a list of output_fields for each class. You can now pass <Class>:<output_fields>;<Class>:<output_fields>;....

{
   "operation": "core/get",
   "class": "Ticket",
   "key": "SELECT UserRequest UNION SELECT Change",
   "output_fields": "Ticket:ref;UserRequest:ref,status,origin;Change:ref,status,outage"
}

I've also added tests for the RestUtils::GetFieldList to ensure backwards compatibility.

Limitation and possible further improvement: For now you need to provide a field list for each each class that can be returned and also the requested class in the "class" parameter. Perhaps this could be changed to have a base list, which would be extended for all child classes, and then override those lists for specific classes when provided in the list.

For now you must also provide the parents (e.g. "Ticket") output_fields, even though no object of parents type (e.g. "Ticket") could ever be returned directly.

Checklist before requesting a review

• [x] I have performed a self-review of my code
• [x] I have tested all changes I made on an iTop instance
• [x] I have added a unit test, otherwise I have explained why I couldn't
• [x] Is the PR clear and detailed enough so anyone can understand digging in the code?

Checklist of things to do before PR is ready to merge

• [ ] Consider different syntax
• [ ] Check if Column Load could be optimized in this case

[jf-cbd]

Hello, thanks a lot for this PR ! We have a few notices on it : The return JSON wouldn't be "standard", different objects can return different fields depending on the requests, it will hard to parse it. Then the API request will return a different result than the OQL query, which can lead to some confusion. We'll discuss our REST API standards and give you some feedback.

[delassiter]

Hi, thanks for the feedback. This is still very much WIP and I agree with your concern.

[jf-cbd]

Hello, thanks for understanding :) Accepted in functional review since it's a really good and useful improvement, we'll check on technical review the format, the documentation to write (e.g. concerning performance), and some other points (ex : do we want it for all operations or just the "get" one)

[jf-cbd]

Hi, we prefer the format you said is currently not possible to do. This format is easier and more intuitive than the one suggested:

{
   "operation": "core/get",
   "class": "Ticket",
   "key": "SELECT UserRequest UNION SELECT Change",
   "output_fields": "ref,status,finalclass"
}

Could you please make some edits to support this syntax instead of the one you proposed ?

First, If one of the output_fields isn't present in any of the selected (sub)classes, an error should be thrown. Then, for each returned object, the selected fields will be return if they exist

Limitation (unlike your proposal) : there wouldn't be the possibility to retrieve a field only from one class and not from another one.