openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG][Go] anyOf with objects and arrays generates uncompilable model

Open MarcelGosselin opened this issue 3 years ago • 2 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [x] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

When a schema has anyOf whose entries are either a plain object or an array, the generated code contains invalid field names in the model.

Actual output:

type Any struct {
	[]Any *[]Any
	map[string]interface{} *map[string]interface{}
}

Expected output (not necessarily an exact match):

type Any struct {
	arrayOfAny *[]Any
	plainObject *map[string]interface{}
}
openapi-generator version

6.0.1

OpenAPI declaration file content or url

OpenAPI with objects and arrays

Generation Details

Running the following without config file, from a folder containing only the file openapi.json

docker run --rm -it -v $(pwd):/src openapitools/openapi-generator-cli:v6.0.1 generate -i /src/openapi.json -g go -o /src
Steps to reproduce
  1. Generate using above command line
  2. Look at file model_any.go, it contains invalid property names []Any and map[string]interface{}
    type Any struct {
        []Any *[]Any
        bool *bool
        float32 *float32
        int32 *int32
        map[string]interface{} *map[string]interface{}
        string *string
    }
    
Related issues/PRs
  1. Similar one with oneOf
  2. More far fetched one with names starting with numbers
Suggest a fix

The file modules/openapi-generator/src/main/resources/go/model_anyof.mustache uses #anyOf which is a Set<string> in CodegenModel.java. Unfortunately in that case the string is a datatype that cannot be used as a field name. I can see 3 ways to fix this (my favorite is the third one):

  1. (I don't know if it is possible in Mustache) In the mustache template, instead of
    {{#anyOf}}
    {{{.}}} *{{{.}}}
    {{/anyOf}}
    
    you could have something to normalize the identifier
    {{#anyOf}}
    {{{ toIdentifier(.) }}} *{{{.}}}
    {{/anyOf}}
    
  2. Change the data type of the Set of CodegenModel.anyOf to be a class which contains 2 properties: DataType, IdentifierName. Then modify usages of anyOf in model_anyof.mustache and any other places to use the right property.
  3. Use type definitions in Go anytime you encounter an array or object, something like the following.
    type openApiObject map[string]interface{}
    type arrayOfAny []Any
    
    You could then use that typedef in the Set<string> which would make the generation give this instead
    type Any struct {
        arrayOfAny *arrayOfAny
        bool *bool
        float32 *float32
        int32 *int32
        openApiObject *openApiObject
        string *string
    }
    

MarcelGosselin avatar Jul 20 '22 13:07 MarcelGosselin

I'm also running into this. It looks like there has been some modifications to the model_oneof.mustache to accommodate this. It looks like there was a helper/lambda added named lambda.type-to-name, which will replace [] with array_of_ and then after that it replaces [ with map_of_ and finally replaces ] with an empty string.

So in your example, instead of the following:

type Any struct {
	[]Any *[]Any
	map[string]interface{} *map[string]interface{}
}

It would produce:

type Any struct {
	array_of_Any *[]Any
	map_of_interface{} *map[string]interface{}
}

You can usage of that lambda in the model_oneof.mustache here.

~However, there are some issues with that still:~

  1. ~Since the first letter is lower case, it not a public property, where all the other properties in the structs are all public.~
  2. ~It uses snake case instead of camel case.~

Edit: Just realized we run it through "camelize", which would convert that appropriately to ArrayOfAny and MapOfInterface{}. Though we'd still need to strip that {} from it though.

ammmze avatar Sep 19 '22 05:09 ammmze

I still need to go through the contributor guidelines and see if we can get any test coverage or whatever this project looks for, but I think something like this should do what we need here.

ammmze avatar Sep 19 '22 05:09 ammmze

Facing the same issue. Any updates??

AniketK-Crest avatar Nov 01 '22 04:11 AniketK-Crest

We're also running into this issue. I replicated the linked PR #13472 locally and confirmed that it fixes the generated Go code. It looks like that PR has passed all automated checks. What can we do to move the review process forward?

ctreatma avatar Nov 10 '22 19:11 ctreatma

I've left a suggestion in https://github.com/OpenAPITools/openapi-generator/pull/13472#issuecomment-1397953579

wing328 avatar Jan 20 '23 05:01 wing328