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

[DefaultCodegen] Incorrect name for request body parameter of array types #376

Closed
etherealjoy opened this issue Jun 21, 2018 · 4 comments
Closed

Comments

@etherealjoy
Copy link
Contributor

etherealjoy commented Jun 21, 2018

Description

When the logs below show up

[main] ERROR codegen.DefaultCodegen - String to be sanitized is null. Default to ERROR_UNKNOWN
[main] ERROR codegen.DefaultCodegen - String to be sanitized is null. Default to ERROR_UNKNOWN

There is an issue in sanitizing the name of the parameter.
For example for aspnetcore the output is below

[SwaggerOperation("CallEndpoint")]
public virtual IActionResult CallEndpoint([FromBody]List<string> ERROR_UNKNOWN)

One of the issues with this is related to how arrays in a request body are handled in DefaultCodegen

public CodegenParameter fromRequestBody(RequestBody body, Map<String, Schema> schemas, 
 Set<String> imports, String bodyParameterName)

the function is called from

public CodegenOperation fromOperation(String path,
				  String httpMethod,
				  Operation operation,
				  Map<String, Schema> schemas,
				  OpenAPI openAPI)

where

String bodyParameterName = "";
if(op.vendorExtensions != null 
&& op.vendorExtensions.containsKey("x-codegen-request-body-name")) 
{
	bodyParameterName = (String) op.vendorExtensions.get("x-codegen-request-body-name");
}
bodyParam = fromRequestBody(requestBody, schemas, imports, bodyParameterName);

and then in fromRequestBody

...
 else if (ModelUtils.isArraySchema(schema)) 
...
while (innerCp != null) {
	if (innerCp.complexType != null) {
		imports.add(innerCp.complexType);
	}
	mostInnerItem = innerCp;
	innerCp = innerCp.items;
}

if (StringUtils.isEmpty(bodyParameterName)) {
	codegenParameter.baseName = mostInnerItem.complexType;
} else {
	codegenParameter.baseName = bodyParameterName;
}
codegenParameter.paramName = toArrayModelParamName(codegenParameter.baseName);

mostInnerItem.complexType is null for arrays of primitive or maps therefore here in toArrayModelParamName sanitization happens with a null resulting in NPE for generators which don't overload toVarName properly.

TL;DR;

Problem is here
codegenParameter.baseName = mostInnerItem.complexType;
below is incorrect

            while (innerCp != null) {
                if (innerCp.complexType != null) {
                    imports.add(innerCp.complexType);
                }
                mostInnerItem = innerCp;
                innerCp = innerCp.items;
            }

            if (StringUtils.isEmpty(bodyParameterName)) {
                codegenParameter.baseName = mostInnerItem.complexType;
            } else {
                codegenParameter.baseName = bodyParameterName;
            }
openapi-generator version

master

OpenAPI declaration file content or url
---
# This is a sample Swagger spec
 
swagger: "2.0"
info:
  description: Container Application Model Description
  version: 1.0.0
  title: ContainerApplication
  
host: localhost:32000
basePath: /api/v1.0
schemes:
- http

tags:  
- name: ApplicationEndpointTest
  x-displayName: "ApplicationEndpointTest"
  description: |
    "Test of endpoints "
    
paths:
  /default/EndpointTest: 
    post:
      tags:
      - ApplicationEndpointTest      
      description: "post a request"
      operationId: callEndpoint
      consumes:
      - application/json
      parameters:
      - name: "requestList"
        in: "body"
        description: "Requests to post"
        required: true
        schema:
          $ref: "#/definitions/listitems"
      responses:
        200:
          description: "Successful response"
        404:
          description: "The request was not successfully executed."
      
  
definitions:
  listitems : 
    description: "list of items"
    type: array
    items: 
      type: "string"
Command line used for generation
java -jar ~/openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar \
generate -i ./api.yaml -g aspnetcore -o genserver 
Related issues/PRs

#373

Suggest a fix/enhancement

This part is handled correctly in case of array of Models but for the primitive and arrays it causes a warning log which can trigger NPE in some generators and causes a wrong parameter name to be created.

@etherealjoy
Copy link
Contributor Author

why can't we use bodyParameterName as a parameter name. It is initialized to "" and not evaluated even though in the spec it is clearly specified.

@jmini
Copy link
Member

jmini commented Jul 3, 2018

Internally we are using Swagger-Parser to read the model. If we read an OAS2, the tool also do an on-the-fly conversion “OAS2 -> OAS3”.

Here is what we get from the parser when we read your example spec:

openapi: 3.0.1
info:
  title: ContainerApplication
  description: Container Application Model Description
  version: 1.0.0
servers:
- url: http://localhost:32000/api/v1.0
tags:
- name: ApplicationEndpointTest
  description: |
    "Test of endpoints "
  x-displayName: ApplicationEndpointTest
paths:
  /default/EndpointTest:
    post:
      tags:
      - ApplicationEndpointTest
      description: post a request
      operationId: callEndpoint
      requestBody:
        description: Requests to post
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/listitems'
        required: true
      responses:
        200:
          description: Successful response
          content: {}
        404:
          description: The request was not successfully executed.
          content: {}
components:
  schemas:
    listitems:
      type: array
      description: list of items
      items:
        type: string

In OAS3 the Request body does not have a name...

I have raised swagger-api/swagger-parser#748 in order to keep this information during the conversion. Feel free to indicate there that you are interested by the feature.

@jmini
Copy link
Member

jmini commented Dec 24, 2018

PR #1721 was merged.

@etherealjoy can you verify if your issue is fixed with the latest 4.0.0-SNAPSHOT?

@etherealjoy
Copy link
Contributor Author

etherealjoy commented Dec 26, 2018

@jmini
I will check and update.
Issue solved in v4.0.x

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

3 participants