Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polymorphism and discriminator for specific requirements #39

Open
GillesInnov35 opened this issue Jan 10, 2024 · 6 comments
Open

Polymorphism and discriminator for specific requirements #39

GillesInnov35 opened this issue Jan 10, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@GillesInnov35
Copy link
Collaborator

GillesInnov35 commented Jan 10, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator Author

GillesInnov35 commented Jan 23, 2024

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
Copy link
Collaborator Author

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 #43
BR
Gilles

@ToshiWakayama-KDDI
Copy link
Collaborator

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
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants