search

sapmentors / cap-community

SAP CAP Community
MIT License
97 stars 26 forks source link

OData V2 Proxy issue with Decimal data type #24

Closed gregorwolf closed 4 years ago

gregorwolf commented 4 years ago

In my example project bookshop-demo I've defined an attribute testDecimal of type Decimal(9,2) in the entity Approval. When I try to update this attribute using the provided OData V4 service it does work without any issues. But when I use the @sap/cds-odata-v2-adapter-proxy i get the error message:

Error while deserializing payload. An error occurred during deserialization of the entity. A JSON number is not supported as Edm.Decimal value.

You can replicate the issue by cloning the issue, and then running:

npm i
npm run build:deploy:startv2

Then use the Fiori App Approval V2 or V4 via the launchpad at http://localhost:4004/app/fiori.html. When you create a new entry in V4 you will have no issues to i.e. enter 3.14 in the Field Test Decimal (9,2). But when you do the same using V2, you should get the error I've described above.

mpah commented 4 years ago

Hi Gregor,

I've run into similiar issues a few weeks back, the solution for me going with @odata.Type : 'Edm.String'; for decimals, then patching the v2 adapter source as follows. +@@ -1028,7 +1028,13 @@ 

         //Skip casting if odata type is string
         if(element["@odata.Type"] && element["@odata.Type"] ==="Edm.String") {
          //skip 
         }
         else{
           data[key] = convertDataTypeToV4(data[key], element.type, headers);
gregorwolf commented 4 years ago

Thank you Maksim. Have you filed an Incident via https://launchpad.support.sap.com/? I have done so now: 26557 / 2020.

gregorwolf commented 4 years ago

As I don't want to change the data type of the field I've tried to implement another solution. Currently this fix:

https://github.com/gregorwolf/SAP-NPM-API-collection/commit/b87f4c0bc3af50dfd20b5067aee7647b615215e8

works for non $batch requests.

gregorwolf commented 4 years ago

As you see in my example I'm using a Fiori Elements UI. I've just checked if there would be an option to set this options in the manifest.json (I had to disable Batch because the header is always sent and destroys the content-type multipart):

    "useBatch": false,
    "headers": {
        "content-type": "application/json;charset=UTF-8;IEEE754Compatible=true"
    },

Unfortunately that results in the issue that instead of a MERGE / PUT a POST is send for the update. Can you please involve someone from the SAPUI5 / OpenUI5 Team to find out the best approach?

oklemenz2 commented 4 years ago

Thanks for reporting this issue. I will have a look, and keep you updated. Oliver

The OData v2 request also works for me (unchanged proxy code), when specifying IEEE754Compatible in content-type: application/json;charset=UTF-8;IEEE754Compatible=trueas header...

Wouldn't that be an option to also pass this content type for v2 requests already from outside as header?

I also checked your fix. And as the OData V4 server only accepts Edm.Decimal and Edm.Int64 with IEEE754Compatible=true, I could think about always setting IEEE754Compatible=true in proxy.

Any reason why you did split "cds.Decimal" apart from "cds.DecimalFloat" in your fix?

"cds.DecimalFloat" and "cds.Decimal" are mapped to "Edm.Decimal" and according to OData V4 spec need to be serialized as string...

oklemenz2 commented 4 years ago

Ok, I see, I'm also not aware of how to change the content-type to include "IEEE754Compatible". I will investigate further...

oklemenz2 commented 4 years ago

A fix will be provided with @sap/cds-odata-v2-adapter-proxy 1.4.16 tomorrow...

oklemenz2 commented 4 years ago

npm show @sap/cds-odata-v2-adapter-proxy

@sap/cds-odata-v2-adapter-proxy@1.4.16 | SEE LICENSE IN developer-license-3.1.txt | deps: 4 | versions: 8 OData v2 Adapter Proxy for CDS OData v4 Services

dist .tarball: https://npm.sap.com/@sap/cds-odata-v2-adapter-proxy/-/cds-odata-v2-adapter-proxy-1.4.16.tgz .shasum: 33ddf800105dedc07b2ba1782612196e00142fe8 .integrity: sha512-O6KI6yrTKeDmu3gOaqGTPq1NcwrNSnlok8ewUaBAwkWWyKQi/eBn5VXMlFa4WTWKOMEgXQRf1CyGxcTyQWBrzQ==

dependencies: @sap/logging: ^5.1.0 axios: 0.19.0 express: ^4.17.1 http-proxy-middleware: ^0.20.0

maintainers:

dist-tags: latest: 1.4.16

published an hour ago by https-support.sap.com do.not.reply@sap.com

oklemenz2 commented 4 years ago

Please have a look and provide feedback. Thx.

gregorwolf commented 4 years ago

I've tested my demo app and my productive app. Both work fine now.