search

camaraproject / KnowYourCustomer

Repository to describe, develop, document and test the KnowYourCustomer API family
Apache License 2.0
9 stars 9 forks source link

Polymorphism and discriminator for specific requirements #39

Open GillesInnov35 opened 8 months ago

GillesInnov35 commented 8 months ago

Use of dataType discriminator or polymorphism with schemas inheriting to address specific attributes for countries.

  1. Define a dataType attribute to be discriminator

Discriminator

  1. Define a common request body with all common attributes

Discriminator

  1. Define specific schemas For example define a common international schema and a specific schema for japaneese requirements

Discriminator

ToshiWakayama-KDDI commented 8 months ago

Hi @GillesInnov35 ,

Thank you for the proposal.

It may work well, but it may make API definition more complex, and I am not quite sure if it is easy to use. Just a comment.

Thanks, Toshi

StefanoFalsetto-CKHIOD commented 8 months ago

I think polymorphism is the right way to have multiple country-specific attributes in a single spec. Just a couple of notes:

  1. The name "DataType" is for me misleading (in my mind it is connected to the concept of "kind of data to be represented in memory" like Integer, String, etc.). I propose to use the name ISO639-1 or language and to use the values defined in ISO639-1 two letter (e.g., EN, ES, PT, IT, etc.)
  2. Naming convention: I think we can assume a more readable approach by putting the language at the end of the structure name, for instance: change from KYC_MatchRequestJapaneseBody to KYC_MatchRequestBody_JP
  3. We currently don't know, but in future some of the attributes in "international" part could be overwritten by some local implementation. Hence, instead of using "international" I propose to use the term "common" or "base".
  4. About the way to allow a Partner to understand which is the specific implementation an MNO is using: After thinking about it, I came to the conclusion that I was wrong, during the phone call. I mean: there is no problem at all. The API call is the last step of a long chain of agreements to be signed and actions to be performed. So when a Partner calls an API knows exactly which is the language used by the OpCo and the which is the composition of the YAML to be used.
GillesInnov35 commented 8 months ago

Thanks @StefanoFalsetto-CKHIOD, my bellow comments

  1. The name "DataType" is for me misleading (in my mind it is connected to the concept of "kind of data to be represented in memory" like Integer, String, etc.). I propose to use the name ISO639-1 or language and to use the values defined in ISO639-1 two letter (e.g., EN, ES, PT, IT, etc.)

I agree with you, dataType is not appropriate to what it should meaning. I'm not sure that language should be used. In our case what we want to distiguish are specific attributes which not only depend on language but also on countries (for example Netherlands requirements). WDYT ?

  1. Naming convention: I think we can assume a more readable approach by putting the language at the end of the structure name, for instance: change from KYC_MatchRequestJapaneseBody to KYC_MatchRequestBody_JP

Yes,I agree, distinction could be placed at the end (suffix ).

  1. We currently don't know, but in future some of the attributes in "international" part could be overwritten by some local implementation. Hence, instead of using "international" I propose to use the term "common" or "base".

Yes, , common is the right term

GillesInnov35 commented 5 months ago

hi @all, do you think we could move forward on this issue in order to address specific countries' attributes.

We'll start to implement KYC Match on our side and the project team considers that specific attributes (such as japaneese attributes) may not be proposed as they're not common attributes but mandatory for some countries.

What do you think about usage of polymorphism ? Could we consider this for Meta release of september ? It's already implemented in one CAMARA's API.

Corresponding proposal for discussion in PR https://github.com/camaraproject/KnowYourCustomer/pull/43 BR Gilles

ToshiWakayama-KDDI commented 5 months ago

Hi @GillesInnov35 , all,

Thanks very much, Gilles, for your further comment. Sorry for the late comment.

From KDDI side, I have discussed internally with our product team. We really appreciate your proposal in this issue, but our feeling is that it could make the API rather complexed. Actually, we have not received any proposal for additional attributes so far. I don't think currently there is any problem with the current plain listing of the attributes, and, as all attributes are optional, you can just omit any attributes you do not need.

Also as you pointed out in another issue (Issue #75), Meta release timing is coming soon, and we need test specifications and some other things to do, so, I think it may be better to keep v0.1.0 as it is and to choose v0.1.0 for the September Meta release. This may be Issue #75 topic, though.

Best regards, Toshi KDDI

GillesInnov35 commented 5 months ago

hi @ToshiWakayama-KDDI , thanks a lot for your feedback. I really think this is not complex and in line with Open API Specifications 3.0 known by much developers. I could help MNO to expose only attributes for which they are concerned. BR Gilles