SAP / cloud-sdk-js

Use the SAP Cloud SDK for JavaScript / TypeScript to reduce development effort when building applications on SAP Business Technology Platform that communicate with SAP solutions and services such as SAP S/4HANA Cloud, SAP SuccessFactors, and many others.
Apache License 2.0
162 stars 55 forks source link

Consider Fractions of Seconds Properly in OData V4 #2608

Closed FrankEssenberger closed 2 years ago

FrankEssenberger commented 2 years ago

Currently we assume three digits for the fractional seconds in the ODataV4 converte. However, this is not correct since the specification states: oasis->link to w3 times:

[0..5][digit].[S+]

So any number of digits after the two leading ones. We need to adjust the generator to:

The value for the percision is given on property level:

<Property Name="DraftEntityCreationDateTime" Type="Edm.DateTimeOffset" Precision="7"/>

The proper solution would be to parse the percision information and add it to the field in the generated client. This is a bigger change I guess. However, adjusting the Edm.DateTimeOffset -> DateRepresentation to accept other percisions then 3 should be easy.

For the other direction we would need the information on the field to send the proper format.

drvup commented 2 years ago

Hi Frank, the related OSS incident belongs to me and I created a custom deSerializer as a workaround. This logic is going to delete the nano seconds from the value of the ABAP Environment oData v4 endpoint, due to the fact, that javascript Date type isn't able to handle nano seconds anyway. So why saving it?

My approach:

import * as sdkOdataCommon from '@sap-cloud-sdk/odata-common/internal';
import moment from 'moment';

const dateTimeDeSerializer: sdkOdataCommon.DeSerializer<moment.Moment> = {
    deserialize: (val: string) => {
        // > in: 2022-06-09T12:55:16.530807Z
        const valWithoutNanoSeconds: string = val.slice(0, -4) + 'Z';
        // < out: 2022-06-09T12:55:16.530Z
        return moment(valWithoutNanoSeconds)},
    serialize: (val: moment.Moment) => val.toString()
};
export const customMomentDeSerializers = { 'Edm.DateTimeOffset': dateTimeDeSerializer };

Hope it does clarify it, adjusting the regex you're using as default should be fine from my POV.

Thanks and kind regards, Cedric

FrankEssenberger commented 2 years ago

Hello @drvup,

great that you found the custom DeSerializer as a mean to overcome the problem as a workaround. I was afraid, that the backend system would not accept a payload with an incorrect number of fractional seconds. Even if in JS they are not correctly handled. So if the EDMX specifies 4 digits and you POST a string like:

1989-01-01T14:23:59.123

it would reject it. I have not tried that to be honest, but this could depend on the system S/4, SFSF, Concour, Ariba.... I would like to have a solution where the number of digits is right.

Best Frank

FrankEssenberger commented 2 years ago

@florian-richter I also heard back from the OData expoert colleagues and the default precision would be 7.

florian-richter commented 2 years ago

@drvup: I implemented a fix to this issue in #2660 and it was published in v2.6.0 of the SAP Cloud SDK. Our default implementation will now behave similarly to your workaround and truncate the partial seconds (because moment and Date don't support any higher precision partial seconds). If you have time to try the new version, feel free to let us know if this allows you to remove your workaround.