RestCode / WebApiProxy

MIT License
199 stars 91 forks source link

New configuration structure for C# client #79

Open faniereynders opened 8 years ago

faniereynders commented 8 years ago

Referring to features/issues mentioned in #10, #14, #35, #58 and #73 -

We need to think of a better structure for the C# client generator configuration file. Here is some of the aggregated features to serve as a starting point:

onionhammer commented 8 years ago

I don't see a reason to allow un-trusted SSL; if the developer wants cant they just disable certificate validation globally (during development)? This seems out of scope to me.

faniereynders commented 8 years ago

@onionhammer I tend to agree, but reading more into issue #35, @pecord might also have a valid point there, given that a server may be only using HTTPS. It's a kind of "chicken or egg" scenario.

@TechnoDezi what do you think?

TechnoDezi commented 8 years ago

@faniereynders I think it should be up to the developer, they should have the option to trust all certificates

faniereynders commented 8 years ago

@TechnoDezi what about the scenario where code generation isn't even possible due to an API hosted only through HTTPS and has an untrusted self-signed certificate? Surely this behavior should be "forced" given this scenario?

faniereynders commented 8 years ago

@onionhammer how does one disable the certificate validation globally?

onionhammer commented 8 years ago

Something like this

ServicePointManager.ServerCertificateValidationCallback = delegate { return true; }; 
faniereynders commented 8 years ago

In my opinion, given a scenario where the service metadata is only accessible via HTTPS, it is obviously for a reason, so whoever is generating code from the metadata needs sufficient access to a proper trusted SSL certificate. In the case where that isn't possible per se, then the proxy endpoint exposing the metadata should just simply be served over HTTP instead.

Having said that, the code will be generated in both scenarios, but will endpoint execution might fail if an HTTPS API endpoint is then called via HTTP. In this scenario we could just include an optional protocol flag in the config so that the client doesn't use he default proxy endpoint's protocol, but the one specified instead?

TechnoDezi commented 8 years ago

If someone is hosting an Api with forced SSL and they using a a self signed cert a protocol overwrite will still break.

faniereynders commented 8 years ago

Let's put this on the back burner for now and continue discussing the other tasks listed on this issue so we can going with a roadmap going forward.

faniereynders commented 8 years ago

The current config file structure, as defined here is:

1-1 proxy [
    1-1 endpoint:uri*, 
    0-1 generateOnBuild:bool, 
    0-1 clientSuffix:string, 
    0-1 namespace:string,
    0-1 name:string,
    0-1 generateAsyncReturnTypes:bool
    ]

To accommodate multiple API services, the following suggested changes need to be made to the structure above:

  1. Introduce a singular root object containing all common properties shared across all API services.
  2. Remove generateOnBuild and generateAsyncReturnTypes from proxy level to the root.
  3. Rename endpoint to proxyEndpoint
  4. Rename proxy to service
  5. Change cardinality from 1-1 to 1-* of service (formerly proxy) into property services

This will result in this structure:

0-1 generateOnBuild: bool,
0-1 generateAsyncReturnTypes: bool,
1-* services: service [
        1-1 proxyEndpoint:uri*
        0-1 clientSuffix:string, 
        0-1 namespace:string,
        0-1 name:string*
    ]

Interested in what we could differently. Your thoughts?

TechnoDezi commented 8 years ago

I agree with this, looks good. This will give us much better flexibility for multiple services.

faniereynders commented 8 years ago

Great. Furthermore we could then also include on the service level includeValidation to pull down any Validation DataAnnotations and ensureSuccess to throw an exception if the response is not 200 OK:

0-1 generateOnBuild: bool,
0-1 generateAsyncReturnTypes: bool,
1-* services: service [
        1-1 proxyEndpoint:uri*
        0-1 clientSuffix:string, 
        0-1 namespace:string,
        0-1 name:string*,
        0-1 includeValidation:bool
        0-1 ensureSuccess:bool
    ]
faniereynders commented 8 years ago

Please see updated JSON schema here

Note that the following properties per service are now required, to accommodate multiple API services:

lust4life commented 8 years ago

@faniereynders i think we should not support DataAnnotations , the model generate for client is just for data transfer, not for validation.

faniereynders commented 8 years ago

Validations are also important. Surely the bare basic validation logic like required, numbers, regex etc will be useful on client side to prevent a chatty API.

It could be generated the same way as the structure of the API gets generated

lust4life commented 8 years ago

i know it can be done though the same way , and validation are surely important. but client site (which use api proxy) should do its own validation, they should have their own modle for ui (check/display), when call an api proxy, they mapper (e.g. automapper) them to models which proxy generate,

and if we give a bare basic validation support, i think more and more (CustomAttr,Description...) will come after. :stuck_out_tongue:

if the validation logic needs to be reuse in each side , they can shared them in another way. but i think the client proxy is just responsible for a http call with typed model.

if we want prevent a chatty api, i think we need think some other way like API Gateway Pattern...

sorry for my english, wish you can understand : )

faniereynders commented 8 years ago

Agreed. It is also definitely possible to at least expose overridable validation functions that will be the same as the code one would've written initially anyway. On Tue, 7 Jun 2016 at 13:00, $+J notifications@github.com wrote:

i know it can be done though the same way , and validation are surely important. but client site (which use api proxy) should do its own validation, they should have their own modle for ui (check/display), when call an api proxy, they mapper (e.g. automapper) them to models which proxy generate,

and if we give a bare basic validation support, i think more and more (CustomAttr,Description...) will come after. 😛

if the validation logic needs to be reuse in each side , they can shared them in another way. but i think the client proxy is just responsible for a http call with typed model.

if we want prevent a chatty api, i think we need think some other way like API Gateway Pattern...

sorry for my english, wish you can understand : )

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/faniereynders/WebApiProxy/issues/79#issuecomment-224247620, or mute the thread https://github.com/notifications/unsubscribe/AB2S38RoKlFvuWQMVgPpZ1sTjEw2Gsafks5qJU8ugaJpZM4HHoHp .