Schema name conflict causing getter and setter ( {schemaName}_{propertyName}able ) interfaces to not be generated

[matthew-c-hpe]

What are you generating using Kiota, clients or plugins?

API Client/SDK

In what context or format are you using Kiota?

Linux executable

Client library/SDK language

Go

Describe the bug

I am trying to generate a client from an OpenAPI spec which is referencing some schemas. One schema is named "{name}Config", the other is named "{name}". The schema named "{name}" has a property called "config". When a schema named "{name}Config" is referenced and used in generation from the bundled spec, Kiota fails to generate the interface "{name}_configable".

A minimal reproduction of the bug showing a failing spec and a working spec can be found here:
https://github.com/matthew-c-hpe/kiota-bug-name-conflict

Note that this behaviour is not limited to the word "Config" and a property named "config". It seems to affect any clashing name/property. See examples in repo above using "Foo" name suffix and "foo" property which illustrates this.

Expected behavior

I expect the client to generate successfully, with the "{name}" schema containing the "config" property to generate an interface "{name}_configable. I expect this behaviour because {name} and {name}Config are named differently and therefore should not conflict.

How to reproduce

Given a schema named "{name}" referenced in a path, 
which has a property named "config",
and another schema named "{name}Config" referenced in the same path,
Kiota will fail to generate the interface {name}_configable for the schema which has the "config" property.

i.e.:

https://github.com/matthew-c-hpe/kiota-bug-name-conflict/blob/main/spec-minimal-bad-bundle.yaml

Open API description file

https://github.com/matthew-c-hpe/kiota-bug-name-conflict/blob/main/spec-minimal-bad-bundle.yaml

Kiota Version

1.17.0

Latest Kiota version known to work for scenario above?(Not required)

No response

Known Workarounds

Rename the conflicting schema to something that doesn't end in "Config".

Configuration

Ubuntu 22.04 WSL2 Windows 10

Debug output

Click to expand log

``` # kiota generate -l go --openapi spec-minimal-bad-bundle.yaml --ll Debug dbug: Kiota.Builder.KiotaBuilder[0] kiota version 1.17.0 info: Kiota.Builder.KiotaBuilder[0] loaded description from local source dbug: Kiota.Builder.KiotaBuilder[0] step 1 - reading the stream - took 00:00:00.0076336 dbug: Kiota.Builder.KiotaBuilder[0] step 2 - parsing the document - took 00:00:00.1091051 dbug: Kiota.Builder.KiotaBuilder[0] step 3 - updating generation configuration from kiota extension - took 00:00:00.0001610 dbug: Kiota.Builder.KiotaBuilder[0] step 4 - filtering API paths with patterns - took 00:00:00.0046948 info: Kiota.Builder.KiotaBuilder[0] Client root URL set to https://foo/api/v1 dbug: Kiota.Builder.KiotaBuilder[0] step 5 - checking whether the output should be updated - took 00:00:00.0189930 dbug: Kiota.Builder.KiotaBuilder[0] step 6 - create uri space - took 00:00:00.0028335 dbug: Kiota.Builder.KiotaBuilder[0] InitializeInheritanceIndex 00:00:00.0032624 dbug: Kiota.Builder.KiotaBuilder[0] CreateRequestBuilderClass 00:00:00 dbug: Kiota.Builder.KiotaBuilder[0] MapTypeDefinitions 00:00:00.0032392 dbug: Kiota.Builder.KiotaBuilder[0] TrimInheritedModels 00:00:00 dbug: Kiota.Builder.KiotaBuilder[0] CleanUpInternalState 00:00:00 dbug: Kiota.Builder.KiotaBuilder[0] step 7 - create source model - took 00:00:00.0634717 dbug: Kiota.Builder.KiotaBuilder[0] 36ms: Language refinement applied dbug: Kiota.Builder.KiotaBuilder[0] step 8 - refine by language - took 00:00:00.0372007 dbug: Kiota.Builder.KiotaBuilder[0] step 9 - writing files - took 00:00:00.0417460 info: Kiota.Builder.KiotaBuilder[0] loaded description from local source dbug: Kiota.Builder.KiotaBuilder[0] step 10 - writing lock file - took 00:00:00.0128246 Generation completed successfully Client base url set to https://foo/api/v1 dbug: Kiota.Builder.KiotaBuilder[0] Api manifest path: /home/matthew/repos/matthew-c-hpe/kiota-bug-name-conflict/apimanifest.json ```

Other information

Have not tested this with generation of clients for all languages.
At the least, it seems to affect client generation in Java too.

[baywet]

Hi @matthew-c-hpe ,
Thank you for using kiota and for reaching out.

After looking at the very detailed issue and the provided repro, let me attempt to rephrase to make sure I understand the problem properly here:

Given that:

• We have a type named XY
• We have another type named X with a property which has an inline schema named Y

And depending on the naming conventions kiota is enforcing for the language, as well as whether the language supports nested type declarations (CSharp) or not (Go): we end up with colliding names in kiota's CodeDOM. Is this correct?

I guess the most straight forward way to deal with such a collision is when we create types definitions for the inline types, if the name is already in use, we could append "Property" instead.

Is this something you'd like to submit a pull request for provided some guidance?

[matthew-c-hpe]

Hi @baywet

Thanks for getting back to me. Yes, you have understood and summarised the problem exactly.

I took a look at a minimal example of C# generated code based on my repo I linked above and it seems that C#'s support for nested classes means that code generation in that language avoids the same kind of naming conflicts as we encounter in Go.

I think that appending "Property" to names already in use is a valid solution or at least a quick fix for this, but I have reservations about it because doing so would mean that interfaces could be named differently to what one may expect. The appending of "Property" would be applied inconsistently - only during clashes, which with a larger API could get confusing for developers.

I'm not sure how big of an issue this is given that they're generated and not exactly meant to be user-facing. If this solution was chosen, I'm wondering if appending "Property" to the name of an inline type's property/properties should be done by default rather than just on clashing names.

I'd like to know what you think about this.

I would be willing to have a go at submitting a pull request for this given some guidance, providing it's for a solution that you and your team are happy with.

[baywet]

Thank you for the additional information.

The challenge to doing it consistently is that it'll be a source breaking change for people who didn't run into the conflict.

1. generate with kiota's current version, no conflict
2. implement a dependency on the property type. (assertion or whatnot)
3. we make the change with the Property suffix.
4. update with kiota's next version.
5. source is broken, code need to be updated.

This goes against our charter/versioning policy.

Does that make sense?

Let us know if you have any additional comments or questions.

[matthew-c-hpe]

That makes sense.

In that case, I will have a go at implementing the solution of appending "Property" to name conflicts when creating type definitions for inline types as suggested.

If you don't mind, if there are any additional resources or pointers you may be able to refer me to to help kickstart diving into the Kiota code base outside of the Kiota documentation and design documentation on the Microsoft website, I would appreciate if you could let me know.

Thank you for your help and prompt communication.

[baywet]

Thank you for your patience.
We recently had a similar pull request you might want to look at to get some more context.
#5312

As the issue is with the conversion to wrappers. This is most likely where the fix will live.

kiota/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs

Line 478 in cdb0c9c

private static CodeType ConvertComposedTypeToWrapper(CodeClass codeClass, CodeComposedTypeBase codeComposedType, bool usesBackingStore, Func<string, string> refineMethodName, bool supportsInnerClasses, string markerInterfaceNamespace, string markerInterfaceName, string markerMethodName)

And you can duplicate this unit test to set the conditions to what is problematic.

kiota/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs

Line 312 in cdb0c9c

public async Task ConvertsUnionTypesToWrapperAsync()

Let us know if you have any additional comments or questions.

[microsoft-github-policy-service]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.