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

[Proposal] Improve the return type for client generation to handle error status code details and extra details #6099

Closed
lnash94 opened this issue Feb 22, 2024 · 4 comments · Fixed by ballerina-platform/openapi-tools#1667

Comments

@lnash94
Copy link
Member

lnash94 commented Feb 22, 2024

Summary

A well-drafted API specification is essential for outlining the responses to all API operations. Each operation should have a defined response, typically focusing on a successful outcome. However, users may also be interested in details regarding response headers and error status codes.

This redesign aims to effectively manage error status codes and header details across all responses during the client generation process.

Consider the following scenario:

This given operation has two responses, one is 200-ok with successful payload User and the header details and the second one is 404 - Not found error response with header details and string body payload.

[1]OAS sample:

/users/{id}:
   get:
    ...
     responses:
       "200":
         description: Ok
          headers:
           api-version:
             required: true
             schema:
               type: string
           user-key:
             required: true
             schema:
               type: string
         content:
           application/json:
             schema:
                 $ref: '#/components/schemas/User'
       "404":
         description: NotFound
         headers:
           api-version:
             required: true
             schema:
               type: string
           user-key:
             required: true
             schema:
               type: string
         content:
           text/plain:
             schema:
               type: string

Goal

  • Improve code generation in the return types to have all details(headers, error status code, payload) in response to client resource/remote function.

Motivation

As outlined in the summary, users may have an interest in deeper into the response beyond its payload. However, in the current code generation setup, our provision is limited to providing a successful payload[2].
[2]Current code sample for the above OAS[1]:

...
resource isolated function get users/[int id]() returns User|error {
       string resourcePath = string `/users/${getEncodedUri(id)}`;
       User response = check self.clientEp->get(resourcePath);
       return response;
   }
...
// Access above resource function 
User res = check userClient->/users/'1

Here[2],

  • It returns only User payload details and drops the response header details
  • Drops the 404 response details and response header details.

With this improvement users can access to the response header details, and error status code details instead of its success response.

Proposed solution

Introduce the typedescriptor as the target type in the function to capture all possible response types and make it external. The end user will receive details about the external function to invoke a specific API.

[3]Expected ballerina code with improvement for the given OAS sample[1]:

type User record {|
    string id;
    string name;
    string address;
|};

type Headers record {|
    string api\-version;
    int user\-key;
|};


// expected success details record for 200 including headers
type UserFound record {|
    *http:Ok;
    User body;
    Headers headers;
|};

// expected error details record for response 404
type UserNotFound record {|
    *http:NotFound;
    string body;
    Headers headers;
|};

public isolated client class Client {
    final http:Client clientEp;
...

      //This is external function 
         resource isolated function get users/[int id](typedesc<User|UserFound|UserNotFound> targetType = <>) returns targetType|error = @java:Method {
             'class: "io.ballerina.openapi.core.generators.client.ExternalClientFunction",
             name: "getResource"
         } external;
      

      //This is implementation of the actual resource
         private isolated function processGetUsersId(string path, map<string|string[]>? headers, typedes<User|UserFound|UserNotFound> targetType) returns
                 User|UserFound|UserNotFound {
	      //Handle payload binding through the new client
             return self.clientEp->get(path, targetType = targetType);
         }
...
}

Variations of this status code response access:

  • Binding the successful payload directly, this support won't break the current behaviour
 User res = check userClient->/users/'1;
  • Binding the successful response directly with header details
 UserFound res = check userClient->/users/'1;
  • Binding only the 404 - Not Found. If the user is only concerned about the non-success response
UserNotFound res = check userClient->/users/'1;
  • Binding all response details the user has provided.
UserFound|UserNotFound res = check userClient->/users/'1;
  • Binding the other status code responses as http:Response (this is extended support)
User|UserFound|UserNotFound|http:Response res = check userClient->/users/'1;

To fulfil the above support we required a fix from the HTTP module with HTTP status code response binding support in the HTTP client (#6100)

High-level design implementation

[4]The flow of remote/resource function call
Screenshot 2024-02-22 at 09 41 54

  • Given one OAS operation has two ballerina functions which are the external function and its implementation function.

Introducing CLI flag option to generate client code with new improvement

bal openapi -i --mode client --generate-return-with-error-code

Above diagram[4], java native implementation is to access external functions. This Java native implementation is done inside the openapi repository and will publish a native jar to the WSO2 nexus. This Java implementation should be accessed by adding it to the Ballerina.toml

Example for Ballerina.toml:

[[platform.java17.dependency]]
groupId = "io.ballerina.stdlib"
artifactId = "openapi-native"
version = "<version>"

Limitation:

  • The current openapi CLI doesn’t get the awareness of the execution environment, whether the execution occurred inside the Bal package or not. Therefore, we are unable to provide this improvement if the user generates code in the non-bal directory.

Solution:

  • This feature improvement will be enabled by triggering a flag in the openapi CLI command during code generation.
  • If the user provides the flag with the CLI, we can then modify the Ballerina.toml with the necessary native dependencies. (If it is not within a Bal package, we can default to the previous code generation process without this improvement. )

Note: this --generate-return-with-error-code flag is a kind of sample name

Risks and Assumptions

https://docs.google.com/document/d/1MczQ7tYdgkiOVMA5XtHUCIFWRDASWDJJV7NSQh1ag58/edit#heading=h.ljlipgffytpf

Dependencies

https://docs.google.com/document/d/1MczQ7tYdgkiOVMA5XtHUCIFWRDASWDJJV7NSQh1ag58/

@lnash94 lnash94 self-assigned this Feb 22, 2024
@lnash94 lnash94 moved this to Todo in OpenAPI Tool Roadmap Feb 27, 2024
@lnash94 lnash94 changed the title [WIP][Proposal] Improve the return type for client generation to handle error status code details and extra details [Proposal] Improve the return type for client generation to handle error status code details and extra details Feb 27, 2024
@lnash94
Copy link
Member Author

lnash94 commented Mar 14, 2024

Meeting: 2024/03/14
Attendees : @shafreenAnfar , @TharmiganK , @dilanSachi , @lnash94

During the stand-up meeting we discussed the support with http:Response along with the union as the target type
Here we discussed

  • Providing support for the user-defined status codes will be handled through the fix and http:Response support on a future request.

@TharmiganK
Copy link
Contributor

Blocked on this: ballerina-platform/ballerina-lang#42404

@TharmiganK
Copy link
Contributor

The implementation changes with the above PR: ballerina-platform/openapi-tools#1667

  1. Introduce a private annotation: MethodImpl to represent the connection between the external function and its implementation function:

    # + return - Ok 
    @MethodImpl {
        name: "getAlbumsImpl"
    }
    resource isolated function get albums(string genre = "Rock",
        typedesc<Album[]|OkAlbumArray|NotFoundErrorMessage|BadRequestErrorPayload> targetType = <>) 
            returns targetType|error = @java:Method {
        'class: "io.ballerina.openapi.client.GeneratedClient", 
        name: "invokeResourceWithoutPath"
    } external;
    
    private isolated function getAlbumsImpl(string genre, 
        typedesc<Album[]|OkAlbumArray|NotFoundErrorMessage|BadRequestErrorPayload> targetType) 
            returns Album[]|OkAlbumArray|NotFoundErrorMessage|BadRequestErrorPayload|error {
        string resourcePath = string `/albums`;
        map<anydata> queryParam = {"genre": genre};
        resourcePath = resourcePath + check getPathForQueryParam(queryParam);
        return self.clientEp->get(resourcePath, targetType = targetType);
    }
  2. This is supported for both client remote and resource methods

  3. The --with-status-code-binding flag will enable this behaviour. And the following will be added to the Ballerina.toml when the configurations are added with bal openapi add command:

    [[tool.openapi]]
    id = "oas_client_api_openapi"
    filePath = "api_openapi.yaml"
    options.statusCodeBinding = true

@shafreenAnfar
Copy link
Contributor

@lnash94 @dilanSachi @TharmiganK @shafreenAnfar had a discussion on this feature and concluded a few things.

  • The new flag should only generate status code bindings as below.
resource isolated function get albums(string genre = "Rock",
    typedesc<OkAlbumArray|NotFoundErrorMessage|BadRequestErrorPayload> targetType = <>) 
  • This means when developing connectors, we need two types of connectors. Simple version of the connector which we have now. Then the advanced version of the connector which we will have with the aforementioned flag. The new connector will be added as follows. For instance consider, gsheet. It will have a new submodule called advgsheet. The new submodule will include the advanced connector with status code bindings. This is assuming that simple connector covers 80% scenario whereas advanced connector support 20% scenario. Also, this will maintain backward compatibility as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment