jcailan / cdse

CDS Extension for External Service Consumption
MIT License
9 stars 6 forks source link

return response as cds does #3

Closed marcosvega91 closed 4 years ago

marcosvega91 commented 4 years ago

Hi @jcailan,

thanks for the library :). It is very useful because cds has different limit. One of the limit is the language. I want to call SAP system using the user language. With cdse I can do using the headers.

I have noticed that the response is different from the traditional run function of cds. To have the same response I have to map the response from axios request removing __metadata like below:

 this.on('READ', CountrySet, async (request) => {
    const response = await serviceWithLang.run({
      url: '/CountrySet',
      headers: {
        'Accept-Language': request.user.locale.toUpperCase(),
      },
    })
    return response.d.results.map(({__metadata, ...rest}) => rest)
  })

Do you think that it can be a default behaviour for the library?

jcailan commented 4 years ago

Hi @marcosvega91

Thanks for submitting a suggestion here!

If I understand your requirement, you wanted to strip out the response for all the metadata information that comes from OData. If I make this requirement catered by CDSE, then we are moving back to being opinionated like in the CAP standard framework. The idea behind CDSE is to make it less opinionated and be more flexible in performing request and response over a REST call. Also, CDSE doesn't even distinguish if it is an OData protocol or just any other regular REST API. Other REST API won't have the metadata information, that's the reason behind it.

Do you agree?

marcosvega91 commented 4 years ago

Yes of course. I think that it should be flexible of course but maybe add some feature that cds has.

For example I can do service.tx(request).run(request.query) and forward my request to the external service. This is very fast if you want to use the microservice like a gateway.

I think that the library maybe can be flexible like it is but add some standard features that can be very useful.

jcailan commented 4 years ago

There's a lot of features that come out-of-the-box from just this one line of code: service.tx(request).run(request.query) what kind of standard feature are you referring to?

marcosvega91 commented 4 years ago

I'm referring to filters, sorts, limit. How can I pass all request information to the external service?

btw thanks for the help :)

jcailan commented 4 years ago

Oh, then that means you are referring to the OData $filter, $orderby, $top, and $skip. I'm afraid this won't make it to CDSE as well because these are OData specific query parameters. The library is not meant to cater just for OData, the original intention is to cater to generic restful API services, not just OData. :)

marcosvega91 commented 4 years ago

ok thanks :). I'll close the issue

marcosvega91 commented 4 years ago

At the end I have done in this way using the cds service

this.on('READ', CategoryGroupSet, (request) => {
  const transaction = service.tx(request)
  transaction.context.headers = {
    'accept-language': request.user.locale.toUpperCase(),
  }

  return transaction.run(request.query)
})