Skip to content
This repository was archived by the owner on Aug 8, 2018. It is now read-only.

Model name is generated from description for body params and responses #76

Closed
arabold opened this issue Nov 3, 2015 · 14 comments
Closed

Comments

@arabold
Copy link

arabold commented Nov 3, 2015

I must admit that I'm new to Swagger and AWS Gateway, but the following seems to be counter-intuitive. Please correct me if I'm doing something wrong or if I'm misunderstanding some core principles here.

I'm creating an API that returns a list of objects. For that reason I'm creating a definition like this:

get:
  summary: Genres
  responses:
    '200':
      description: Result
      schema:
        type: array
        items:
          $ref: Genre

The respective model definition looks like this:

Genre:
  type: object
  properties:
    genreId:
      type: string
    genreName: 
      type: string

Import works fine. However, the model created in AWS API Gateway is named "Result" now - it's based on the description of the response. Having multiple responses with the description "Result" will result in conflicts with only a single model being defined in the end. That can't be right, can it?

My expectation would be that I can define the name of the model, e.g. by specifying the "name" parameter.

I've identified the root cause of my problem in the ApiGatewaySdkSwaggerApiImporter.java as follows:

    String generateModelName(Response response) {
        return generateModelName(response.getDescription());
    }
    private String generateModelName(BodyParameter param) {
        return generateModelName(param.getDescription());
    }

For API responses as well as request bodys, the description property is used to generate the model name. However, for BodyParameter the property name seems more appropriate in my opinion. For Response I haven't found an obvious property such as name, but maybe a vendor extension would work.

Oh, and the most curious part is that the actual description field of the model in API Gateway (the one visible in the AWS console) is never set.

Any thoughts?

@etzlers
Copy link

etzlers commented Nov 3, 2015

+1 I've noticed this as well and agree that name would make more sense.

@rpgreen
Copy link
Contributor

rpgreen commented Nov 3, 2015

I agree name should be used when available. When not available we revert to using the description field as the name. If you notice any places where name could be used instead of description please send a pull request - thanks!

@stevben
Copy link

stevben commented Mar 31, 2016

In your example, do you know if the 'Result' model is an array of 'Genre' ? When importing this kind of configuration, we have a third model created (by the importer) called 'Result_item', not 'Genre'. This is very annoying. Maybe this is linked with you issue ? (the use of the description field as model name)

We created an issue in secure pet-store example here : aws-samples/api-gateway-secure-pet-store#11

@balakataws
Copy link

@stevben: I verified the above example as well as the secure-pet-store and a third model wasn't created. Can you provide a minimal swagger (by exporting the API stage) which has this problem?

Also, in your API Gateway API, do you have the third model?

@stevben
Copy link

stevben commented Apr 2, 2016

@balakataws My bad, I forgot to mention that I was talking about models in Android/iOS SDKs generated through the Api Gateway Service. This is in fact a "aws-swagger-codegen" issue. There is not a third model in the API, but it appears only when you export the clients.

How to reproduce.

  1. Create an API with a YAML (the attached YAML is derived from https://github.com/awslabs/api-gateway-secure-pet-store/blob/master/src/main/resources/swagger.yaml with MOCK Data).
    `./aws-api-import.sh --create ~/Desktop/API Gateway Secure Pet Store-test-swagger-integrations.yaml``
  2. Create a stage
  3. Export SDK iOS/Android.

Files :
API Gateway Secure Pet Store-test-swagger-integrations.yaml.txt
Result :
capture d ecran 2016-04-02 a 02 07 26

@stevben
Copy link

stevben commented Apr 2, 2016

@balakataws Note that our current fix is to use swagger-codegen with custom code for AWS authentication.

@balakataws
Copy link

@stevben I checked this. This seems to be a bug in aws-apigateway-importer. We are already working on a better version of import feature, so this bug will be fixed soon.

The issue is that, even though the swagger definition of "Pets" refers to "Pet", the API Gateway model "Pets" (created during import) doesn't refer to the model "Pet". Instead, it contains a copy of the definition of "Pet". That's why the exporter or SDK generator create another model called "PetItem". So, this is just an issue with importer; not an issue with exporter or SDK generator.

There is a work-around. Once you import, the "Pets" model in API Gateway API will look like this:

{"type":"array","items":{"$ref":"#/definitions/Pet"},"definitions":{"Pet":{"properties":{"petId":{"type":"string","description":"The generated unique identifier for the new pet"},"petType":{"type":"string","description":"Free text pet type"},"petName":{"type":"string","description":"Free text pet name"},"petAge":{"type":"integer","description":"Age of the new pet"}}}}}

You will need to modify it to:

{"type":"array","items":{"$ref":"https://apigateway.amazonaws.com/restapis/<YOUR_API_ID>/models/Pet"}}

Then you can generate SDK or export without a third model.

@arabold
Copy link
Author

arabold commented Apr 4, 2016

@balakataws , that's what Pull Request #81 will do for you automatically (use external models instead of inlining). However, the PR is open since November with zero feedback whether it would ever be merged or not.

@balakataws
Copy link

@arabold Sorry for the delay on that pull request. We will work on pending pull requests soon.

@stevben
Copy link

stevben commented Apr 4, 2016

Thanks.

FYI, our workaround was to use swagger-codegen instead which handles correctly this example despite the fact that the swagger importer made a copy. The authentication with swagger-codegen requires a small changes in order to communicate with Amazon Api Gateway (see RiiotLabs/swagger-codegen@d91b9d5)

More generally, is there any advantage of using the library exported by Amazon vs Swagger-Codegen which supports much more languages ?

@sapessi
Copy link

sapessi commented Apr 4, 2016

Apologies for the delay guys. I'm going to go through all of the changes today, resolve the conflicts, and merge them.

@rpgreen
Copy link
Contributor

rpgreen commented Apr 4, 2016

@arabold Thanks for bringing this to our attention. I've added some feedback to #81

@stevben If swagger-codegen is working for your use-case I would encourage you to use it. The advantage of using API Gateway SDKs is that API Gateway-specific features will be added to the clients going forward, but at current time there is minimal difference. I would encourage you to contribute your AWS signature changes back to swagger-codegen if possible

@rpgreen
Copy link
Contributor

rpgreen commented Apr 6, 2016

Swagger/OpenAPI import is now generally available in the API Gateway REST API, the AWS CLI and all AWS SDKs. You can also import and export Swagger definitions using the API Gateway console. This release addresses many of the open issues and feedback in this repository.

I would encourage you to migrate your workflow to the standard AWS tools. aws-apigateway-importer will receive minimal support from the API Gateway team going forward.

Any feedback or issues with the new importer should be directed to the official API Gateway forums.

Thanks,
Ryan

@rpgreen rpgreen closed this as completed Apr 6, 2016
@stevben
Copy link

stevben commented Apr 6, 2016

Very cool.

However, I receive 'Invalid Swagger 2.0 input' when trying to import our swagger.
Note that :

Is there any possibility to have more details about the errors at importation ?
Should I use the SDK JS to get them ?

Thanks

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

No branches or pull requests

6 participants