-
Notifications
You must be signed in to change notification settings - Fork 40
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
Change Request: Standardize OperationId's of paths in OpenApi definitions of FSPIOP API. #78
Comments
Thanks @kleyow! 2 questions come to mind:
|
After researching a bit. Here are two suggestions in my descending preference. More pragmatic version similar to what we are using. Could get verbose with long paths.example. camelCase {HTTPVerb} {Resource} By {Param} {Param} {Param}...etc {HTTPVerb} /participants/{Type}/{ID}/{SubId}/error putParticipantsByTypeIDSubIdError or participantsByTypeIDSubIdErrorPut More descriptive human readable Id.GET a single Model: getModel example /participants/{Type}/{ID}/{SubId}/error notifyParticipantsByTypeIDSubIdError
For internal Mojaloop development
For external parties it should be a nice to have at best. Minimal impact since their code is based off their own local openapi definition they can still use whatever OperationId they want. I believe. |
I think this suggestion is great @kleyow! Direct Comments:
In General:I would take this a step further, generalise this and extend it to all our APIs. For example on FSPIOP PUT operations are akin to a notification as you infer above. But in the Admin API, the PUT is a classic "update" operation, and thus I would add this to the "general" list:
Edge Cases:There is another point on the FSPIOP API that we would need to address here: specifically, the "PATCH" operation added in FSPOP v1.1 --> https://github.com/mojaloop/mojaloop-specification/blob/master/documents/v1.1-document-set/fspiop-v1.1-openapi2.yaml#L1818 In this kind of situation, I do feel that it starts to get ugly and thus it may be cleaner to stick to HTTP Methods as the prefix instead. However, this would then require the user to infer the semantics of the method based on the context (i.e. FSPIOP vs Admin API). Example: I don't have a problem with either naming approach, but I do feel that the later is simpler. |
Thanks for the issue @kleyow and the comments @lewisdaly and @mdebarros ... So @kleyow, in our context, what are the frameworks that are using operationIDs? Even though this is a definite nice to have, we need to validate if this is a requirement at this stage or a standardization for future purposes. Also, as some of you have stated, wanted to put this here for anyone concerned - this change doesn't affect any existing or current implementations and should be non-breaking because the current implementations wouldn't have relied on these (operation IDs) anyway.. |
@elnyry-sam-k openapi-backend is one that is currently in our stack. https://github.com/mojaloop/transaction-requests-service/search?q=TransactionRequestsByID Here you can see an example of handlers having to be correlated to an OperationId. |
Ok, thanks for that Kevin. lets setup some time to go over these. |
Open API for FSP Interoperability - Change Request
Table of Contents
1. Preface
___This section contains basic information regarding the change request.
1.1 Change Request Information
2. Problem Description
___2.1 Background
The OpenApi definitions of v1.0 and v1.1 of the FSPIOP Specification has no standard naming scheme for operationID's. OperationID's are important for some framworks when routing paths. Some microservices deviate from the hosted definition
2.2 Current Behaviour
Current definition has no standardized naming scheme for operation Id's
Example
https://github.com/mojaloop/mojaloop-specification/blob/master/fspiop-api/documents/v1.1-document-set/fspiop-v1.1-openapi3.yaml#L111
https://github.com/mojaloop/mojaloop-specification/blob/master/fspiop-api/documents/v1.1-document-set/fspiop-v1.1-openapi3.yaml#L237
2.3 Requested Behaviour
Operation Id's are updated.
3. Proposed Solution Options
___Decide on naming scheme and update documents.
The text was updated successfully, but these errors were encountered: