Skip to content

REST: Allow class based output fields + Allow OQL with multiclass result #708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

accognet
Copy link
Contributor

@accognet accognet commented Apr 1, 2025

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
Proposed change makes it possible to optionally define a list of output_fields for each class. You can now pass
:<output_fields>;:<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"
}

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.
Another solution is to pass all fields and returned classes:

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

This 2 call return exactly the same result.


Another case:
The following is currently not possible:

{
   "operation": "core/get",
   "class": "Ticket",
   "key": "SELECT P, PR, F
  FROM Person AS P
  JOIN Person AS PR ON P.manager_id = PR.id
  JOIN lnkContactToFunctionalCI AS L ON L.contact_id = P.id
  JOIN FunctionalCI AS F ON L.functionalci_id = F.id",
   "output_fields": "friendlyname, email, organization_name,  move2production"
}

Now this kind of call return the same results as in screen


This PR merges the code of #668 + code send by a client.

@CombodoApplicationsAccount CombodoApplicationsAccount added the internal Work made by Combodo label Apr 1, 2025
@accognet accognet self-assigned this Apr 1, 2025
@github-project-automation github-project-automation bot moved this to First review needed in Combodo PRs dashboard Apr 1, 2025
@accognet accognet moved this from First review needed to Pending functional review in Combodo PRs dashboard Apr 1, 2025
@accognet accognet changed the title Feature/rest multi classes REST: Allow class based output fields + Allow OQL with multiclass result Apr 1, 2025
@jbostoen
Copy link
Contributor

jbostoen commented Apr 1, 2025

The idea is nice; but why work with comma separated values?
Perhaps an associative array could be provided for the output fields; with the class name being the key and the value an array of attribute codes?

If the JSON structure is pretty printed, it would look nicer this way :)

@jf-cbd jf-cbd moved this from Pending functional review to Pending technical review in Combodo PRs dashboard Apr 4, 2025
@Molkobain
Copy link
Contributor

Nice job, I'm very interested by the second use case! 🤩
Like Jeffrey, I would rather have a way to specify output fields for each classes, like it's done in \DBObjectSet::OptimizeColumnLoad().

@Hipska
Copy link
Contributor

Hipska commented Apr 14, 2025

Yup, I agree..

To keep consistency with original implementation maybe it would look like this?

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

But it looks like @jbostoen and @Molkobain are hinting to this:

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

@jbostoen
Copy link
Contributor

An array of strings just makes more sense to me in terms of APIs, rather than concatenating values first.

The original format was perhaps not optimal to begin with in this aspect.

I'm also missing a point in the example.

From the original proposal (it might just be the example):

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

What would the purpose/objective be of 'Ticket' here?
If it's meant to list attributes on parent classes that we want; I understand; but why repeat it for the specific classes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Work made by Combodo
Projects
Status: Pending technical review
Development

Successfully merging this pull request may close these issues.

5 participants