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

MP-25: Cover ExtensionList Mojaloop type with test cases. #126

Closed
wants to merge 2 commits into from

Conversation

kirgene
Copy link
Contributor

@kirgene kirgene commented Jan 20, 2020

No description provided.

@kirgene kirgene changed the title Cover ExtensionList Mojaloop type with test cases. MP-25: Cover ExtensionList Mojaloop type with test cases. Jan 20, 2020
Copy link
Contributor

@bushjames bushjames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirgene see my comment on the inbound API change.

description: Data model for the complex type ExtensionList
properties:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirgene : You cannot change this API, it is the actual Mojaloop API which is strictly controlled. I know having an array property "extension" in a property called "extensionList" is a bit crazy but that is the way it has been defined by the API specification. We cannot arbitrarily change it or we will break compatibility with every mojaloop API implementer.

Copy link
Contributor Author

@kirgene kirgene Jan 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bushjames I thought that I adhered to the specs. For example, on page 30 (paragraph 3.3.4.3) there is an example of an error object, and on page 136 (paragraph 7.4.2) you can see the description of ErrorInformation type where extensionList is also described.
From what I understood, there is a confusion between actual key names and just placeholders in type description tables.
For example: on page 93 (paragraph 6.5.2.2) there is a spec for POST /quotes body and you can see the quoteId key is of type CorrelationId. Then if you follow the description of type CorrelationId on page 130 (paragraph 7.3.8) you can see there is a key CorrelationId that is of type UUID.
So according to the specs the value of quoteId should be the following:

{
  ...
  quoteId: {
    CorrelationId: "00000000-0000-1000-8000-000000000002"
  }
  ...
}

but in reality we have the following:

{
  ...
  quoteId: "00000000-0000-1000-8000-000000000002"
  ...
}

Copy link
Contributor

@bushjames bushjames Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @kirgene , yes the document is sometimes not clear and can be confusing for sure. There are no examples in the API spec document to clarify the extensionList structure. However, there are examples in the swagger yaml, for example: https://github.com/mojaloop/sdk-scheme-adapter/blob/master/src/InboundServer/api.yaml#L147 which I have taken as definitive. I suggest we stick to the examples there and ask for clarification from the API control board.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirgene I have sent an email to the chair of the API control board asking for clarification.

Copy link
Member

@elnyry-sam-k elnyry-sam-k Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirgene @bushjames - I translated (on behalf of the Change Control Board (CCB) ) the API Definition which was in word format into the Swagger / OpenAPI (v1.0 of the Spec) that we're now using across projects (and in the mojaloop core). Specifically for the exetensionList element, I used the definition in the Data Model as you rightly indicated, from sections 7.4.2 - 7.4.4. I've consulted about this with the board at the time and it went through reviews there. You're correct that a few examples are not following that definition and its on the list of changes that will come with a v1.1 of the Specification doc.

Regarding the question of exact translation of items such as QuoteId, you'll notice that the yaml from here is named implementation friendly because when this discussion about IDs being objects (type CorrelationId), it was decided at the time, in the group to use a simpler implementation friendly yaml thats more straight forward and uses the regular expressions directly instead of using objects. However, we'll be revisiting this (at the CCB) when the version v1.1 release as well, whether or not to use a strict translation this time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bushjames
Copy link
Contributor

closing as reimplemented in another PR without change to API structures.

@bushjames bushjames closed this Mar 2, 2020
@elnyry-sam-k elnyry-sam-k deleted the feature/MP-25 branch September 16, 2020 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants