pimcore / data-hub

Data delivery & consumption platform for Pimcore.
Other
127 stars 109 forks source link

Interface implementation types returned as empty objects in lists #206

Closed PMarci closed 4 years ago

PMarci commented 4 years ago

I've been trying queries with DataHub for an integration project and noticed what seems to be incorrect behavior when querying with inline fragments. Here's a query that asks for properties on a trivial DataObject class with type property_text image an empty object appears in the list, presumably because of the way resolvers for different property types work. Here's the Properties tab of the object in question: image

Reading through the GraphQL documentation, it seems to me like this contradicts the intended behavior of GQL APIs:

Note that name is still specified on Starship because otherwise it wouldn't show up in the results given that Starship is not a Character!

And for good reason, having rogue empty elements appear in arrays complicates processing of the returned data. Am I correct in my interpretation that this is, in fact, a bug?

PMarci commented 4 years ago

This is on DataHub commit dev-master#4401634 but seen as this affects DataObjects I suspect it's much the same on master as well.

PMarci commented 4 years ago

Works the same on 0.6. image image

dvesh3 commented 4 years ago

@PMarci This is not a bug. Either use keys argument to filter properties or define all property types https://github.com/pimcore/data-hub/blob/master/doc/graphl/querysamples/Sample_ElementProperties.md#request

PMarci commented 4 years ago

Based on the GraphQL documentation one would think that defining the implementing type of the ElementProperty interface would be the filtering, and that the inline fragment should prevent empty objects from appearing. If one is forced to have prior knowledge of the properties defined on a particular object in order to avoid empty ElementProperty objects from showing up, that precludes many avenues of processing an arbitrary set of properties that a DataObject might have. Or at least without checking every object for emptiness. The alternative of defining every type (and at least one field for each!) in the query is also sub-optimal, because one might not be interested in them, I understood the point of GraphQL as having the ability to avoid such unnecessary overheads.

PMarci commented 4 years ago

Scratch that, it would seem this query from the demo behaves the same way. image So it seems we'll have to just live with this behavior.