APIM 3.1.0 Admin API Documentation improvements and suggestions

wso2 / docs-apim

Apache License 2.0

70 stars 395 forks source link

Description:

POST /throttling/policies/application

When we are adding an Application Throttling Policy, in the payload policyId is there which is not needed because, after the creation in the response we will get a different policyId. So, this policyId can be removed from that request Payload window in the documentation[1]. Also, in the Curl window, the URL has v1 instead of v0.16. That also should be changed as follows. https://127.0.0.1:9443/api/am/admin/v0.16/throttling/policies/application

Further, when we are adding a policy with a name which is already added, we are getting the following response.

{
    "code": 409,
    "message": "Resource Already Exists",
    "description": "Application Policy with name 201PerMin already exists",
    "moreInfo": "",
    "error": []
}

This regarding the above 409 response code there is no explanation in the Responses section. It would be great if it is considered.

PUT /throttling/policies/application/{policyId}

Regarding the Description of the policy, it can be null even if we try to update or create a policy via UI too. Adding Description as nullable would be better in the documentation[2].

POST /throttling/policies/subscription

customAttributes which is to be added to the Subscription Throttling Policy is an array type, where it should be an object/hashtable. There was no example like below is not provided.

"customAttributes":          [
               {
      "name": "customAttr1",
      "value": "value1"
   },
               {
      "name": "customAttr2",
      "value": "value2"
   }
]

rateLimitTimeUnit was not described with its allowed values whether we can use null or NA or any other allowed values.
Allowed values of the timeUnit are mentioned as "sec", "min", "hour", "day". But, sec is not allowed via UI. Therefore, we can remove it from the documentation. And other Allowed values like a week, month and year through UI is not mentioned here. Please update the proper values of them in the Allowed values section.
Here also, the policyId is not needed in the request template. We can remove it.
Again the following response code 409 was not mentioned in the response section.
```
{
"code": 409,
"message": "Resource Already Exists",
"description": "Subscription Policy with name sri already exists",
"moreInfo": "",
"error": []
}
```
PUT /throttling/policies/subscription Regarding the Description of the policy, it can be null even if we try to update or create a policy via UI too. Adding Description as nullable would be better in the documentation[4].

POST /throttling/blacklist

The conditionId is not needed to be mentioned in the request template. This is the same thing as the policyId in the above scenarios.
Condition types which are API Context, Application, IP Address, IP Range, User not defined with their allowed values in the documentation and are hard to extrapolate from the info in the UI everytime.
Condition value can be more descriptive similar in the UI as below.
```
Format : ${userName}
Eg: admin
```
The provided example of IP condition is invalid, and not even proper JSON as below. "conditionValue": "{\"fixedIp\":\"192.168.1.1\":\"invert\":false}"

POST /labels [5] POST /api-categories [6]

Regarding the above 2, The id is not needed to add the label and API categories through the REST call.

PUT /labels/{labelId} The second thing the id is not required to be sent in the request body and it does not make sense with the labelId already existing, and it is in the path parameter. Additionally, the request example specifically states this: "id": "This is not mandatory.Auto generate by code" as well. 'id' is not required, can be omitted, and if it is sent, it is ignored. Therefore, it does not need to be documented in the request parameters.[7]

GET /applications Here the statement says "If no user is provided then the application for the user associated with the provided access token will be returned" But, we can change it with more information as below. If no user query is provided, the application(s) for the user associated with the Authorization Header will be returned PUT /api-categories/{apiCategoryId} Regarding the id, since that is already in the path param, it is no need to be mentioned in the request template. [9]

Affected Product Version: APIM 3.1.0

Hi @riyazathali,

We have gone through the pointed issues.

Description:

POST /throttling/policies/application

When we are adding an Application Throttling Policy, in the payload policyId is there which is not needed because, after the creation in the response we will get a different policyId. So, this policyId can be removed from that request Payload window in the documentation[1].

POST /labels [5] POST /api-categories [6]

Regarding the above 2, The id is not needed to add the label and API categories through the REST call.

PUT /labels/{labelId} The second thing the id is not required to be sent in the request body and it does not make sense with the labelId already existing, and it is in the path parameter. Additionally, the request example specifically states this: "id": "This is not mandatory.Auto generate by code" as well. 'id' is not required, can be omitted, and if it is sent, it is ignored. Therefore, it does not need to be documented in the request parameters.[7]

Yes, the ID fields are auto-generated internally and the parameter passed in the request will not be honored when creating/editing the policies/labels/api-categories. We can mark these parameters are read only by adding readOnly: true in API definition, similar to [1].

Also, in the Curl window, the URL has v1 instead of v0.16. That also should be changed as follows. https://127.0.0.1:9443/api/am/admin/v0.16/throttling/policies/application

Rest API version should be changed to v0.16

Further, when we are adding a policy with a name which is already added, we are getting the following response.
{
    "code": 409,
    "message": "Resource Already Exists",
    "description": "Application Policy with name 201PerMin already exists",
    "moreInfo": "",
    "error": []
}

Seems the resource definition - POST /throttling/policies/application is missing the description of response code 409 and 500 response codes. We need to verify it for the other resources as well and add the relevant response definitions.

PUT /throttling/policies/application/{policyId}

Regarding the Description of the policy, it can be null even if we try to update or create a policy via UI too. Adding Description as nullable would be better in the documentation[2].

The application policy can be created with or without specifying the description field. It is not marked as required true in API definition. However, the support to add nullable attributes is introduced in OAS 3.x version (Refer [2] and [3]). Since the 3.1.0 Rest APIs are based on Swagger 2.0 and it doesn't affect the functionality we can have it as it is for now. Please create an issue in product-apim to resolve it in future release. Please note that REST APIs are not used in UIs in 3.1.0. It uses the Jaggery APIs for Admin portal.

1. customAttributes which is to be added to the Subscription Throttling Policy is an array type, where it should be an object/hashtable. There was no example like below is not provided.

Custom Attributes are correctly defined in Request Body schema - [4]. We need to add the same in Sample payload as well.

1. rateLimitTimeUnit was not described with its allowed values whether we can use null or NA or any other allowed values.

2. Allowed values of the timeUnit are mentioned as  "sec", "min", "hour", "day". But, sec is not allowed via UI. Therefore, we can remove it from the documentation. And other Allowed values like a week, month and year through UI is not mentioned here.  Please update the proper values of them in the Allowed values section.

We would need to test and verify the allowed values and add it under the description of each field. It would be better if we can have enums for this. Since we cannot introduce such changes to the existing definitions, we can change these fields as enums in future release. Please create a git issue for this too to track it for future releases.

3. Condition value can be more descriptive similar in the UI as below.

Format : ${userName}
Eg: admin

1. The provided example of IP condition is invalid, and not even proper JSON as below.
   `"conditionValue": "{\"fixedIp\":\"192.168.1.1\":\"invert\":false}"`

We need to provide examples for each conditionType similar to [5].

GET /applications Here the statement says "If no user is provided then the application for the user associated with the provided access token will be returned" But, we can change it with more information as below. If no user query is provided, the application(s) for the user associated with the Authorization Header will be returned

We will update the description more clear.

[1] https://github.com/wso2/docs-apim/blob/3.1.0/en/docs/develop/product-apis/devportal-apis/devportal-v1/devportal-v1.yaml#L3613 [2] https://stackoverflow.com/questions/48111459/how-to-define-a-property-that-can-be-string-or-null-in-openapi-swagger [3] https://swagger.io/docs/specification/data-models/data-types/ [4] https://apim.docs.wso2.com/en/3.1.0/develop/product-apis/admin-apis/admin-v0.16/admin-v0.16/#tag/Subscription-Policy-(Collection)/paths/~1throttling~1policies~1subscription/post [5] https://github.com/wso2/docs-apim/blob/3.1.0/en/docs/develop/product-apis/publisher-apis/publisher-v1/publisher-v1.yaml#L6974

wso2 / docs-apim