[Kotlin] Correct isInherited flag for Kotlin generators

[mtraynham]

Most of the Kotlin generator templates loop requiredVars and optionalVars (1, 2) . Unfortunately, it looks like during the CodegenModel.removeAllDuplicatedProperty, the CodegenProperty's are all cloned and therefore the current implementation of only updating the allVars list isn't sufficient to reflect that in requiredVars or optionalVars.

All 3 lists (allVars, requiredVars, optionalVars) should be updated with the isInherited property.

Also, the KotlinSpringServerCodegen was doing the same thing as AbstractKotlinCodegen so I removed it.

PR checklist

• Read the contribution guidelines.
• If contributing template-only or documentation-only changes which will change sample output, build the project before.
• Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
• File the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
• Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@jimschubert @dr4ke616 @karismann @Zomzog

[mtraynham]

@jimschubert @dr4ke616 @karismann @Zomzog @wing328 any feedback on this would be appreciated!

[jimschubert]

@mtraynham sorry. I've been super busy. I'm trying to get caught up on some issues over the next couple of days.

Good catch on the clone call in CodegenModel.removeAllDuplicatedProperty... that seems a little suspect to me and I can't tell if it was an intentional side effect there or not.

Looking over your code, it looks good and the comments are helpful. I was wondering if it would be possible to check the properties on the "final" model in the test, to avoid regressions? For example, if pm.requiredVars and pm.optionalVars were ever inadvertently truncated or didn't hold exactly the intended properties, it seems this test would still pass.

[mtraynham]

Hey @jimschubert, I'll rebase so there aren't conflicts and add a check to verify that all the names of the properties in requiredVars and optionalVars. Sound good?

[jimschubert]

@mtraynham Thanks! I checked the latest commit and it looks good. We'll merge once CI checks have completed.

[mtraynham]

Awesome, thanks @jimschubert!

I wanted to point out one thing, my test case change also coincides with a separate bug I found, #4253, where the optionalVars would have actually also contained a (requiredVars: a, c; optionalVars: a, b, d). This occurs when adding the requiredItems to the outer model of an allOf composed object and it duplicates parent required items into the optional vars list. If I instead move the required items to child of an allOf it works as you expect. Swagger-jaxrs is generating the former, where requiredItems is a combination of both the child and parent properties that lives on the outer properties of a composed model.

[mtraynham]

@jimschubert @wing328 any chance you guys can take a look at this again?

[mtraynham]

Ping @jimschubert @dr4ke616 @karismann @Zomzog @wing328

[jimschubert]

@mtraynham Thanks for pinging again. The holidays were super hectic for me, sorry for the long delay.

I appreciate that you merged with latest :)