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

'ballerina swagger client|mock' generates syntactically incorrect ballerina code #11461

Closed
ahochsteger opened this issue Nov 18, 2018 · 13 comments

Comments

@ahochsteger
Copy link

Description:
When generating client or mock modules from a valid swagger spec (e.g. https://petstore.swagger.io/v2/swagger.json) there's generated invalid ballerina code.

Example of the invalid generated code in petstore-mock:

gen/swagger_petstore.bal:
updatePetWithForm (endpoint outboundEp, http:Request _updatePetWithFormReq, int petId,  _updatePetWithFormBody) {
// Note the missing type of _updatePetWithFormBody

swagger_petstore_impl.bal:
public function updatePetWithForm (http:Request _updatePetWithFormReq, int petId,  _updatePetWithFormBody) returns http:Response {
// Note the missing type of _updatePetWithFormBody

The reason seems to be that "application/x-www-form-urlencoded" is not supprted for the consumed body. Similar errors as in petstore-mock occur in the generated petstore-client code.

Example of the invalid generated code in petstore-client:

    public function init(SwaggerPetstoreClientConfig config) {
        endpoint http:Client httpEp {
            url: config.serviceUrl; // Note the semikolon instead of the comma
            auth: config.clientConfig.auth; // Note the semikolon instead of the comma
            cache: config.clientConfig.cache
        };

Steps to reproduce:

$ mkdir petstore-test
$ cd petstore-test
$ ballerina init
Ballerina project initialized
$ ballerina swagger mock https://petstore.swagger.io/v2/swagger.json -m petstore-mock
successfully generated ballerina mock service for input file - https://petstore.swagger.io/v2/swagger.json
$ ballerina swagger client https://petstore.swagger.io/v2/swagger.json -m petstore-client
successfully generated ballerina client for input file - https://petstore.swagger.io/v2/swagger.json
$ ballerina build
Compiling source
    hello_service.bal
    orgname/petstore-mock:0.0.1
error: orgname/petstore-mock:0.0.1::gen/swagger_petstore.bal:149:115: mismatched input ')'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-mock:0.0.1::swagger_petstore_impl.bal:47:106: invalid token ')'
    orgname/petstore-client:0.0.1
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:22:35: invalid token ';'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:22:35: invalid token ';'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:22:35: extraneous input ';'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:25:9: invalid token '}'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:27:13: extraneous input '.'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:28:13: extraneous input '.'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:88:62: invalid token ','
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:88:62: mismatched input ','. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:88:73: mismatched input ')'. expecting ';'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:88:105: mismatched input '{'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:89:22: mismatched input ':'. expecting ';'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:91:16: extraneous input '.'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:91:51: mismatched input ';'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:94:42: extraneous input '->'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:94:49: invalid token '"/pet/{petId}"'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:94:82: invalid token ')'
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:94:82: mismatched input ')'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-client:0.0.1::gen/swagger_petstore.bal:97:5: mismatched input 'public'. expecting {';', '[', '?', '|'}

Affected Versions:
Ballerina 0.983.0

OS, DB, other environment details and versions:

$ uname -a
Linux <hostname> 4.18.16-arch1-1-ARCH #1 SMP PREEMPT Sat Oct 20 22:06:45 UTC 2018 x86_64 GNU/Linux

Suggested Labels (optional):

  • Component/ToolSwagger
  • Type/Bug
@ahochsteger
Copy link
Author

I was able to track it down to the following issues:

  1. Typos in the generated ClientConfig
  2. Unsupported content type 'application/x-www-form-urlencoded' that is used by the petstore example leads to empty type names in method arguments
  3. Different expected arguments of endpoint methods (e.g. get vs. post/put) lead to syntax errors in method calls

ad.1 Typos in generated ClientConfig
They have already been fixed in this pull request: #11480

ad.2 Missing support of content type 'application/x-www-form-urlencoded':
I suspect the missing pieces should be implemented in BallerinaRequestBody.java#getRequestBType(String mType) to assist the generation of correct ballerina code. If I'm not missing something, the right ballerina type should be 'map' as it is used in Request#getFormParams().

ad.3 Different arguments for endpoint methods
Currently the generated client code always looks like this - no matter which http method is used:

        return check _updatePetEp->put("/pet", request = request);
        return check _addPetEp->post("/pet", request = request);
        return check _findPetsByStatusEp->get("/pet/findByStatus", request = request);
        return check _deletePetEp->delete("/pet/{petId}", request = request);

Here we have two problems:

  1. The second method parameter should be named 'message' instead of 'request'. It seems to have been changed on June 9th as part of a larger sync: 77d804c
  2. The different signature of the endpoint methods is not respected. E.g. get() is expecting the second parameter to be a defaultable parameter which is not the case for put(), post() and delete(), where it is a mandatory parameter. This is the currently working code that should be generated:
        return check _updatePetEp->put("/pet", request);
        return check _addPetEp->post("/pet", request);
        return check _findPetsByStatusEp->get("/pet/findByStatus", message = request);
        return check _deletePetEp->delete("/pet/{petId}", request);

I may be able to provide a pull request for the 3rd issue (both problems) but I guess I will need some help for the 2nd one (support for content type 'application/x-www-form-urlencoded') since it will touch deeper parts of the swagger code generation. Maybe somebody with deeper insights into the swagger code generation (@praminda ?) can have a look at the options?

Last but not least, it would be great to add tests for the swagger code generation functionality to be able to spot problems earlier. Any pointers, where the right place for this kind of tests would be?

@DimalChandrasiri
Copy link
Contributor

Thank you very much for reporting the issue. We were able to recreate the issue and we are working on a fix for it.

@ahochsteger
Copy link
Author

ahochsteger commented Nov 20, 2018

@DimalChandrasiri thanks for confirming :).

I am already working on a fix by adjusting the mustache templates only but there are some things I don't quite understand:

  • Q: What's the purpose of requestBody.selectedMedia.mediaType as it doesn't seem to work anyway (at least for the petstore example)? Is it just a convenience method to provide the correct Ballerina type to the mustache templates?
  • Q: Where do the special mustache expressions {{>pathParams}} and {{>reqBody}} come from?
    • A: They include mustache templates with the given name (e.g. pathParams.mustache).
  • Q: Is there somewhere a documentation of the expressions and the data model you can use in the mustache templates? If not, I'm currently trying to create a README.md capturing the important bits. Would be great, if it could be added somewhere (e.g. to the templates folder).
  • Q: Why is there a mix of objects from io.swagger.v3.oas.models.* and org.ballerinalang.swagger.model.* exposed to the mustache templates (see org.ballerinalang.swagger.model.BallerinaOpenApi for a starting point)? Do the ballerina types extend the OpenAPI types with additional variables for the mustache templates? If the OpenAPI type is wrapped by a Ballerina type, are the original OpenAPI fields still accessible?

Thanks for any feedback,
Andreas

@NipunaMarcus
Copy link
Contributor

NipunaMarcus commented Dec 12, 2018

@ahochsteger
There was bit of new syntax changes we have added with the new release Ballerina v0.990.0. This affected service syntaxes and the endpoint syntaxes hence we had to change our template which used to generate the Ballerina service and client accordingly ... There are few known issues that we need to address (like the issue that you mentioned regarding media types and schemas) and will be address in the next immediate release. To answer few of your questions

1st Q:

will look in to this issue.

2nd Q:

yes we have a separate template file with that name in the same resource folder.

3rd Q:

Unfortunately no. There is no documentation regarding the templates and data models used in them. That is actually a good idea to create a README.md to capture such information. It is okay to keep it in the resources/templates folder.

4th Q:

Actually what we are exposing is ballerina models which wraps OAS models. This has been done because when we generate Ballerina source for a swagger we need some details which are specific to ballerina to be sent along the way. As we have the origin OAS models wrap in the model you can access most of the details which we support to be shown in the annotation level.

@ahochsteger
Copy link
Author

@NipunaMarcus
Thanks for the update and your effort to fix these problems!

I've tested Ballerina v0.990.0 and can still see some issues with a simplified version of the pestore example that produces only application/json instead of both xml+json since this is not yet supported.
You can find the swagger file here: petstore-simple.zip

Generated Mocks:

$ ballerina swagger mock clients/petstore-simple.json -m petstore-simple-mock
successfully generated ballerina mock service for input file - clients/petstore-simple.json
$ ballerina build petstore-simple-mock
Compiling source
    orgname/petstore-simple-mock:0.0.1
error: orgname/petstore-simple-mock:0.0.1::gen/swagger_petstore.bal:148:136: invalid token ')'
error: orgname/petstore-simple-mock:0.0.1::swagger_petstore_impl.bal:47:106: invalid token ')'

The first error message is in the following code (note the missing data type at _updatePetWithFormBody):

    @http:ResourceConfig {
        methods:["POST"],
        path:"/pet/{petId}",
        body:"_updatePetWithFormBody"
    }
    resource function updatePetWithForm (http:Caller outboundEp, http:Request _updatePetWithFormReq, int petId,  _updatePetWithFormBody) {
        http:Response _updatePetWithFormRes = updatePetWithForm(_updatePetWithFormReq, petId, _updatePetWithFormBody);
        _ = outboundEp->respond(_updatePetWithFormRes);
    }

The second error is in this function (again note the missing data type):

public function updatePetWithForm (http:Request _updatePetWithFormReq, int petId,  _updatePetWithFormBody) returns http:Response {
    // stub code - fill as necessary
    http:Response _updatePetWithFormRes = new;
    string _updatePetWithFormPayload = "Sample updatePetWithForm Response";
    _updatePetWithFormRes.setTextPayload(_updatePetWithFormPayload);

	return _updatePetWithFormRes;
}

I guess both still have to do with the unsupported content type 'application/x-www-form-urlencoded'.

Generated Clients
Here I still get a lot of syntax errors:

$ ballerina swagger client clients/petstore-simple.json -m petstore-simple-client
successfully generated ballerina client for input file - clients/petstore-simple.json
$ ballerina build petstore-simple-client
Compiling source
    orgname/petstore-simple-client:0.0.1
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:17:24: mismatched input 'client'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:17:30: mismatched input ';'. expecting 'object'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:22:14: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:37:5: extraneous input 'new'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:37:20: mismatched input '{'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:39:5: mismatched input 'public'. expecting {';', '[', '?', '|'}
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:40:50: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:50:47: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:60:57: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:68:55: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:76:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:83:62: invalid token ','
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:83:62: mismatched input ','. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:83:73: mismatched input ')'. expecting ';'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:83:105: extraneous input '{'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:84:13: mismatched input ':'. expecting ';'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:84:58: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:84:48: extraneous input '.'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:84:57: mismatched input '.'. expecting ';'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:84:64: mismatched input ';'. expecting 'object'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:85:13: mismatched input ':'. expecting ';'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:86:16: extraneous input '.'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:86:27: missing token ';' before '('
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:86:51: mismatched input ';'. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:89:42: mismatched input '->'. expecting ';'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:89:48: mismatched input '('. expecting {'[', '?', '|', Identifier}
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:89:73: extraneous input '='
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:89:82: extraneous input ')'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:90:5: extraneous input '}'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:93:50: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:101:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:110:53: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:118:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:127:53: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:135:52: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:143:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:152:66: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:161:65: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:170:50: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:178:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:186:54: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:194:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:203:51: invalid token 'client'
error: orgname/petstore-simple-client:0.0.1::gen/swagger_petstore.bal:210:1: extraneous input '}'

Here is an example of an affected code snippet (the first error of the list above; problematic lines are marked with comments):

public type SwaggerPetstoreClientEp object {
    public http:Client client; // Syntax error on 'client'
    public SwaggerPetstoreClientConfig config;

    public function init(SwaggerPetstoreClientConfig config) {
        http:Client httpEp = new(config.serviceUrl, config = {auth: config.clientConfig.auth, cache: config.clientConfig.cache});
        self.client = httpEp; // Syntax error on 'client'
        self.config = config;
    }

    public function getCallerActions() returns (SwaggerPetstoreClient) {
        return new SwaggerPetstoreClient(self);
    }
};

It seems to be related to the variable name 'client' that produces errors. If I change that e.g. to 'xclient' many of those errors are gone. Is 'client' a reserved word in the Ballerina language? I did not find anything related in the Ballerina Language Specification.

I hope, this helps to further fix the remaining issues to improve the out-of-the-box experience with ballerina and swagger code generation :-).

Regards,
Andreas

@NipunaMarcus
Copy link
Contributor

thank you @ahochsteger
yes this is a bug which we missed during the testing and yes client template seems to be not updated with the new language syntaxes.

Will address these with the next release.

@ahochsteger
Copy link
Author

@NipunaMarcus, I just retested the code generation with version 0.990.3-rc2 and could see some good progress :-).
The petstore example is now working but I've got still problems with more complex APIs, esp. APIs that contain multiple parameters (path, query params, body, ...) or combinations thereof where invalid ballerina syntax is generated, e.g.:

# Swagger spec:
GET /path/{param1}/{param2}

# Code:
public remote function myFunction(string param1int param2) returns http:Response | error { ... }

# Build error:
extraneous input 'param2'

Another example:

# Swagger spec:
POST /path (with one body parameter referencing a schema type and 4 query parameters of type string)
# Code:
public remote function myFunction(MyRequest _myBody, , , , ) returns http:Response | error {

# Build error:
invalid token ','

I guess this is what most of the generated code from more complex APIs causes to fail.

@NipunaMarcus
Copy link
Contributor

@ahochsteger Thank you very much for testing the swagger tooling .... Yes there are bugs related to the operation parameters when generating the code and there are still some unsupported types... This missing comma is happening due to a mustache template issue ...

In this week we are to have a review and usability test on swagger tooling ... Aim of those is to capture functionality and usability issues in swagger tooling. We are planning to fix as much as possible .... I will reference this issue when we start fixing issues you mentioned ...

@NipunaMarcus
Copy link
Contributor

Related Issue: #13588

@anupama-pathirage anupama-pathirage added the Team/CompilerFETools Semantic API, Formatter, Shell label Apr 30, 2019
@NipunaMarcus
Copy link
Contributor

As we revisited the design of the OpenAPI tooling and the annotation structure, which you can refer from specification available here, we are closing this issue and attaching it to the newly created issue to track the progress and evaluation purpose.

@lafernando
Copy link
Contributor

Hi,

I get similar issues in Ballerina 0.991.0, where this https://raw.githubusercontent.com/OAI/OpenAPI-Specification/master/examples/v3.0/link-example.yaml, causes the following errors when building. Is this something that is fixed now?.

$ ballerina build
Compiling source
    schema.bal
error: .::schema.bal:16:5: redeclared symbol 'repository'
    link_example.bal
error: .::link_example.bal:42:64: extraneous input 'slug'
error: .::link_example.bal:50:78: extraneous input 'slug'
error: .::link_example.bal:58:70: mismatched input 'slugstring'. expecting {',', ')'}
error: .::link_example.bal:58:84: mismatched input ')'. expecting ';'
error: .::link_example.bal:58:116: extraneous input '{'
error: .::link_example.bal:59:13: mismatched input ':'. expecting ';'
error: .::link_example.bal:63:9: extraneous input 'return'
error: .::link_example.bal:63:44: extraneous input '->'
error: .::link_example.bal:63:49: missing token ';' before '('
error: .::link_example.bal:63:125: invalid token ')'
error: .::link_example.bal:63:125: mismatched input ')'. expecting {'[', '?', '|', Identifier}
error: .::link_example.bal:66:5: mismatched input 'public'. expecting {';', '[', '?', '|'}
error: .::link_example.bal:66:67: mismatched input 'slugstring'. expecting {',', ')'}
error: .::link_example.bal:66:81: mismatched input ')'. expecting '='
error: .::link_example.bal:66:113: extraneous input '{'
error: .::link_example.bal:67:13: mismatched input ':'. expecting '='
error: .::link_example.bal:71:9: extraneous input 'return'
error: .::link_example.bal:71:41: extraneous input '->'
error: .::link_example.bal:71:47: missing token '=' before '('
error: .::link_example.bal:72:5: extraneous input '}'

@NipunaMarcus
Copy link
Contributor

@lafernando This is happening due to a generating wrong syntax ... we fixed this in the current milestones and there are few major changes. Anyway we will check this .yaml and let you know ...

@DimalChandrasiri
Copy link
Contributor

The service generation process for the file link_example gets generated successfully in ballerina version 1.0 Beta. Hence closing the issue.

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

No branches or pull requests

6 participants