Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Update API-design-guidelines.md #96

Merged
merged 4 commits into from
Nov 18, 2022
Merged

Update API-design-guidelines.md #96

merged 4 commits into from
Nov 18, 2022

Conversation

RubenBG7
Copy link
Contributor

  • Added versioning control of rest parts involved in Apification projects.
  • X-Correlator architecture header changed to optional from required

@RubenBG7
Update API-design-guidelines.md
08b9858 
- Added versioning control of rest parts involved in Apification projects.
- X-Correlator architecture header changed to optional from required
@shilpa-padgaonkar shilpa-padgaonkar mentioned this pull request Oct 19, 2022
Closed
@shilpa-padgaonkar
Copy link
Contributor

@RubenBG7 : Thanks for extending the versioning section as discussed. Could you kindly update the PR so that the approved merge would add your doc directly to the documentation directory rather than the working directory?

@jordonezlucena jordonezlucena removed the request for review from petorre October 21, 2022 10:15
@GarethWGSMA
Copy link

Hi, could we consider if we need to stipulate if the API's should use camel case, spinal case, other formatting for the uri's ? I think as we're creating a suite of 8 API's some consistency would be useful for resource names, dates and parameters.

I was thinking something like the following:

[https://api.gov.au/sections/naming-conventions.html#:~:text=Query%20Parameter%20Names,-Literals%2Fexpressions%20in&text=Query%20parameters%20MUST%20start%20with,that%20are%20not%20URL%20safe.]

@sachinvodafone
Copy link
Contributor

I would like to understand  if there is any specific reason to not exploring standard error response code  that described in IETF RFC 7807. It seems , there is no direct correlation between section 3.2(HTTP Response Codes) and section 6 (Error Response).

@sachinvodafone
Copy link
Contributor

If we are referring “ Response body validation” under section 10.2 (Security implementation), are we considering OAS validation (OpenAPI Specification Validation)

@sachinvodafone
Copy link
Contributor

Under section 8.1 (Pagination), we are referring different query parameter for fetching records like “per_page” and “page” instead of standard “Offset” and “Limit” respectively (recommended by TMForum). As we generally use keywords “Limit” and “Offset” while fetching records from the database so would recommend the same naming convention so that it can be mapped directly.

SELECT
column_list
FROM
table1
ORDER BY column_list
LIMIT row_count OFFSET offset;

@sachinvodafone
Copy link
Contributor

https://api.gov.au/sections/naming-conventions.html#:~:text=Query%20Parameter%20Names,-Literals%2Fexpressions%20in&text=Query%20parameters%20MUST%20start%20with,that%20are%20not%20URL%20safe.]

I would also recommend that Names in Paths URI (tasks, individual resources, etc.) MUST be camel case or lower case and not "snake_case" that is mentioned in section 4.2

@RubenBG7
Copy link
Contributor Author

@sachinvodafone Please move these questions to the original PR --> #72

@sachinvodafone
Copy link
Contributor

@sachinvodafone Please move these questions to the original PR --> #72

Hi @RubenBG7 It seems , PR --> #72 is closed already so not sure if it will be reviewed again. However I will update it as per suggestion.

@GarethWGSMA
Copy link

@sachinvodafone thanks for your reply. Can I ask why would you favour camel case over the others ? And I would agree that we make our formatting MANDATORY across camara

@sachinvodafone
Copy link
Contributor

@sachinvodafone thanks for your reply. Can I ask why would you favour camel case over the others ? And I would agree that we make our formatting MANDATORY across camara

Hi @GarethWGSMA Thanks for your concern and reply. As per my view, Camel case or Pascal case is probably good for the resource names as it make it easier to read and remember. It is also recommended by TM Forum in its API guideline recommendation . CamelCase is also popular convention in computer programming and variable names. To keep them consistent and readable, it is best practice to establish same naming conventions within an organisation.
However it is not a strict guideline so we can also adopt existing snake case if it's having some pros over camel as per our requirement.

@GarethWGSMA
Copy link

@sachinvodafone Hi, no I concur I don't really mind which format we adopt - but I would have thought it should be mandatory. I've no experience of why there should be any flexibility with the URI format, it doesn't alter the API in anyway. I was leaning towards spinal case (probably biased because I've used it before), but if TM forums have a standard then maybe we should follow it ? I'm hoping others chime in with the preferences and reasons.

@RubenBG7
Copy link
Contributor Author

RubenBG7 commented Oct 31, 2022 •
edited by jordonezlucena
Loading

Hi @sachinvodafone, First of all thanks for your comments.

CAMARA is a new standard of APIs that we are creating to be integrated in the NorthBound interfaces of Telcos to be DEVELOPER oriented. TMForum is a good framework to creates APIs Telco Oriented, but CAMARA is not TMForum and it is not Telco Oriented. The biggest Developer oriented marketplaces as Google, Amazon... doesn't follow the TMForum rules... So we need to understand what are the advantages of camelCase in front of Snake_case and same to the errors format proposed. Our criteria to define the API guidelines must be supported by the four pillars of APIfication:

  • Customer / Developer experience
  • Performance / secured
  • Usability / Good documented
  • Reusability

We are also drafting a one-pager where we explain the position of TM Forum Open APIs in the operator's side when implementing CAMARA APIs. In a nutshell, he telco-oriented approach of Open APIs makes them valuable at operator's internal IT stack (specially regarding the integration of API-GW with BSS/OSS suite), but we think neither their as-is style nor structure is valuable for developers. Nonetheless, we will share this one-pager (via PR) as soon as we get it ready.

This was referenced Oct 31, 2022
Closed
Closed
@eric-murray
Copy link
Contributor

eric-murray commented Nov 2, 2022 •
edited
Loading

I've reviewed the whole proposal with @sachinvodafone, and Vodafone would like to see the following changes to the proposed design guidelines. The remaining proposals are acceptable to us.

  • The HTTP error code should be included in any error response, as the error JSON may be distributed elsewhere or stored without the associated error code. Whilst the status code can be inferred from the "code" itself, this is potentially subjective and an explicit "status" field avoids the possibility of error miss-attribution.

    {
   "status": 400,
   "code": "INVALID_ARGUMENT",
   "message": "A human readable description of what the event represent"
}

  • The filtering options for ranges are clumsy and inflexible, and should be replaced by the TMF notation:
    Greater or equal | GET .../?name.gte=Juan | GET .../?amount.gte=807.24 | GET.../?executionDate.gte=2018-30-05 |
    Strictly greater | GET .../?name.gt=Juan | GET .../?amount.gt=807.24 | GET.../?executionDate.gt=2018-30-05 |
    Smaller or equal | GET .../?name.lte=Juan | GET .../?amount.lte=807.24 | GET.../?executionDate.lte=2018-30-05 |
    Strictly smaller | GET .../?name.lt=Juan | GET .../?amount.lt=807.24 | GET.../?executionDate.lt=2018-30-05 |

    Multiple conditions can be specified to define ranges:
    GET /users?creationDate.gte=2020-01-01T00:00:00&creationDate.lte=2021-11-31T23:59:59
    Search for users created between 2020 and 2021

    URL encoded forms of all the operators should also be accepted
    = %3D%3D, > %3E, >= %3E%3D, < %3C, <= %3C%3D,

@shilpa-padgaonkar shilpa-padgaonkar mentioned this pull request Nov 3, 2022
Closed
@RubenBG7
Copy link
Contributor Author

RubenBG7 commented Nov 8, 2022 •
edited
Loading

Changes Applied with @eric-murray comments treated on last Commonalities call.

@eric-murray
Copy link
Contributor

@RubenBG7 - did you make some changes to the PR? I can't find them. Can you share a link?

@RubenBG7
Update API-design-guidelines.md
7e89e70 
According to VF comments, we update Guidelines in next titles:
6 - Versioning
8.3 - Filtering
@RubenBG7
Copy link
Contributor Author

@eric-murray please check updates on commit: Last Commit for Guidelines

@eric-murray
Copy link
Contributor

@RubenBG7
I'm happy that the update addresses my requested changes.

A final review yielded some comments on the error codes themselves:

  • OAuth failure should be 401, not 403. For me, 401 means "please refresh your OAuth token and try again", whereas 403 means "I understood what you asked me to do, but I'm not going to do it, so don't ask again".
  • 206 and 503 are missing. 206 is mentioned elsewhere (for pagination). 503 is already in use in some of the Swaggers.
  • Also consider adding 429. I don't believe it is any of our Swaggers yet, but it is surely only a matter of time.

@RubenBG7
Update API-design-guidelines.md
8421d50 
Added 206 and 503 https codes
@RubenBG7
Copy link
Contributor Author

@eric-murray

  • Codes 503 and 206 added following RFCs referenced in Guidelines
  • Code 429 is not present in the standard RFCs, When it wil be needed in any contract we will include it in the Guidelines.
  • 401 applied when you have a token problem (INVALID, EXPIRED...) 403 is defined as forbidden action and it will be showed only when your token is active. I don't understand what are you meaning exactly, because the HTTP codes are described same that RFCs indicate.

@GarethWGSMA
Copy link

GarethWGSMA commented Nov 11, 2022 via email

@RubenBG7
Copy link
Contributor Author

@GarethWGSMA

Same that 429 , Code 422 is not present in the standard RFCs, When it will be needed in any contract we will include it in the Guidelines.

@GarethWGSMA
Copy link

GarethWGSMA commented Nov 15, 2022 via email

@eric-murray
Copy link
Contributor

@RubenBG7

I don't understand what are you meaning exactly, because the HTTP codes are described same that RFCs indicate.

I think you need to change the text for 403 errors, as "invalid" is ambiguous. A token can be invalid because it has expired, or the client just used a random token, both of which would be 401. I suggest something like:

"It will be returned when the OAuth token access does not have the required scope or when the user fails operational security."

@RubenBG7
Update API-design-guidelines.md
7b91d79 
text for 403 errors updated to be more specific.
@RubenBG7
Copy link
Contributor Author

@eric-murray

Changes applied to be more specific with 403 code description.

@shilpa-padgaonkar
Copy link
Contributor

As agreed here https://github.com/camaraproject/WorkingGroups/blob/main/Commonalities/documentation/MeetingMinutes/MOM-2022-11-16.md, merging the PR on 18th EOD

@shilpa-padgaonkar shilpa-padgaonkar merged commit 0c23f22 into camaraproject:API-design-guidelines Nov 18, 2022
@eric-murray eric-murray mentioned this pull request Feb 28, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shilpa-padgaonkar shilpa-padgaonkar shilpa-padgaonkar approved these changes

@petorre petorre petorre approved these changes

@eric-murray eric-murray Awaiting requested review from eric-murray

@GarethWGSMA GarethWGSMA Awaiting requested review from GarethWGSMA

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@RubenBG7 @shilpa-padgaonkar @GarethWGSMA @sachinvodafone @eric-murray @petorre