Fix: Escape paths containing the keyword "item"

[CasperWSchmidt]

Fixes #4814

[CasperWSchmidt]

@microsoft-github-policy-service agree

[baywet]

@CasperWSchmidt thanks for starting this. Can you add a unit test in the kiota builder tests please? This way we can iterate on the solution with confidence.

[CasperWSchmidt]

@baywet I'm not sure what unit test to add. I can include a test using the exact structure I'm struggling with, but that seems to be a bit too large for at unit test?

[andrueastman]

@CasperWSchmidt Maybe you can take a look at a test like this one?
You can essentially use a subset of your description as the input and assert that the class and namespace exist with the expected names in the codeDom.

kiota/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs

Line 188 in 285db9f

public async Task HandlesSpecialCharactersInPathSegment()

[CasperWSchmidt]

Hi @andrueastman and @baywet
I'm sorry for the late reply. I've tried adding a unit test that verifies what I expect. I've commented out the Assert that fails with the current implementation. I also believe I'm catching the issue with the original implementation (without the escaping I've done). Please let me know if you need me to do something else/more.

[baywet]

@CasperWSchmidt Thanks for your continuous efforts here.
I found some time to look at your PR and make further progress.
This still needs to be refactored to avoid duplication for constants and conditions. Can you take it from here please?

[CasperWSchmidt]

@baywet Thank you for helping out! I don't know what constants and conditions must be refactored to avoid duplication.

[CasperWSchmidt]

@baywet Thank you for helping out! I don't know what constants and conditions must be refactored to avoid duplication.

Ah, the "Item" and "Item_escaped" constants? What's the best place to put them?

[CasperWSchmidt]

I tried playing around with changing Item_escaped with something else and noticed that my latest changes doesn't include the constants in OpenApiUrlTreeNodeExtensions. When looking into it, I noticed another private const string EscapeSuffix = "Escaped"; I wonder if I could (and should) consolidate this with the other changes to have a single escape suffix across the builder?

[baywet]

I wonder if I could (and should) consolidate this with the other changes to have a single escape suffix across the builder?

Yes, and please place the constants in the Extensions class.

[CasperWSchmidt]

@baywet Thanks, I finally got around to fixing it. I guess it's ready for review/merge.

[baywet]

Thank you for making the changes!

Can you also add a changelog entry please? (unreleased, changed)

[baywet]

Made the changelog entry myself