[ 
https://issues.apache.org/jira/browse/OLINGO-1550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17429223#comment-17429223
 ] 

Daniel Fernández commented on OLINGO-1550:
------------------------------------------

Note that OLINGO-737 is an *additional issue for aggregations in Olingo*.

The results of aggregations need to be output in dynamic properties. For 
instance: "{{Total}}" in "{{aggregate(Amount with sum as Total)}}".

The current implenentation of the JSON/XML serializers requires fields to be 
present in the EntityType, which is not the case for these aggregated fields.

> Aggregations not working due to JSON/XML serializer implementations
> -------------------------------------------------------------------
>
>                 Key: OLINGO-1550
>                 URL: https://issues.apache.org/jira/browse/OLINGO-1550
>             Project: Olingo
>          Issue Type: Bug
>          Components: odata4-server
>    Affects Versions: (Java) V4 4.8.0
>            Reporter: Daniel Fernández
>            Priority: Major
>              Labels: aggregation, groupby, json, serialization, xml
>
> The current implementation of the {{ODataJsonSerializer}} and 
> {{ODataXmlSerializer}} classes contain (at least) two issues that make 
> {{$apply=groupby(...))}} aggregations *unusable* in Olingo.
>  
> +*ISSUE 1: The fields to be output are not properly computed.*+
>  
> The {{writeProperties(...)}} method at the serializer implementations contain 
> this:
> {code:java}
>     final boolean all = ExpandSelectHelper.isAll(select);
>     final Set<String> selected = all ? new HashSet<String>() :
>             
> ExpandSelectHelper.getSelectedPropertyNames(select.getSelectItems());
> {code}
> This helper only checks the presence of the {{$select}} query option and 
> builds the list of fields to be output from that parameter. So if {{$select}} 
> is not present it sets {{all = true}} and considers that all fields in the 
> entity should be output.
> But this is not correct in the case of an aggregation, because only the 
> fields that take part in the aggregation should be output even if {{$select}} 
> is not specified, as can be seen in section ["3.10 Transformation 
> groupby"|http://docs.oasis-open.org/odata/odata-data-aggregation-ext/v4.0/cs02/odata-data-aggregation-ext-v4.0-cs02.html#_Toc435016582]
>  of the spec for the _OData Extension for Data Aggregation Version 4.0_:
> {code:java}
> GET ~/Sales?$apply=groupby((Customer/Country,Product/Name),
>                            aggregate(Amount with sum as Total))
> {code}
> …results in_
> {code:json}
> {
>   "@odata.context": "$metadata#Sales(Customer(Country),Product(Name),Total)",
>   "value": [
>     { "@odata.id": null, "Customer": { "Country" : "Netherlands" },
>       "Product": { "Name": "Paper" }, "Total": 3 },
>     { "@odata.id": null, "Customer": { "Country": "Netherlands" },
>       "Product": { "Name": "Sugar" }, "Total": 2},
>     { "@odata.id": null, "Customer": { "Country: "USA" },
>       "Product": { "Name": "Coffee" }, "Total": 12},
>     { "@odata.id": null, "Customer": { "Country: "USA" },
>       "Product": { "Name": "Paper" },"Total": 5},
>     { "@odata.id": null, "Customer": { "Country: "USA" },
>       "Product": { "Name": "Sugar" }, "Total": 2}
>   ]
> }
> {code}
> So it seems {{ExpandSelectHelper}} should be taking any requested 
> aggregations into account when computing the fields to be output.
>   
> +*ISSUE 2: Primary Key fields are always forced into output.*+
>  
> The same {{writeProperties(...)}} methods of both serializer implementations 
> invoke this just before actually starting the serialization of the fields.
> {code:java}
>     addKeyPropertiesToSelected(selected, type);
> {code}
> So even if a {{$select}} query option had been added to the request, or the 
> above _"issue 1"_ was fixed and not all fields were being selected for output 
> when an aggregation is requested, this would still force the entity's PK 
> fields into output in every case.
> This seems to have been added as a part of the fix for OLINGO-1246.
> But when an aggregation is performed, _fields not taking part of the 
> aggregation will never have a value_, which is why their output should not be 
> attempted. Unless the PK fields are part of the aggregation, there will be no 
> possible values available for these fields because the entity objects they 
> belong to should have been _aggregated_.
> So when this happens, the serializer will obtain a {{null}} value for the PK 
> fields that have been added to the output by 
> {{addKeyPropertiesToSelected(selected, type)}} –and which shouldn't be 
> there–, check that these PK fields are _not nullable_ and throw an exception:
> {code:java}
> SerializerException: Non-nullable property not present!
> {code}
> Lastly, note that I understand that adding the PK fields to every response 
> payload does not go against the spec because, as explained in OLINGO-1246, 
> the spec allows additional fields to be added when {{$select}} is used…  but 
> doing this adds to the overall weight of the payload –especially with complex 
> PKs– and therefore in my opinion should _not_ be a default behaviour.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to