Create valid property identifiers when generating OData client

[gregwinterstein]

Modify the code generator to generate valid identifiers

This change resolves the following issues:

• Dynamics 365 OData metadata contains spaces causing OData generated code to fail compilation #317
• odata-cli generated entity with "Context" property will not compile #297 & Class property Context appears to be defined using the incorrect type of string vs DataServiceContext #331
• [T4] OData v4 Client Code Generator generates a C# proxy class that won’t compile if the model contains an Entity that has a property with name "Context" #242
• Invalid class name generated #224

[SomeTroglodyte]

Simple - this can't be any worse than the current master.

[gregwinterstein]

@microsoft-github-policy-service agree

[mvarie]

When using the ODataConnectedService extension in Visual Studio 2022 to generate C# classes from an unpatched Dynamics 365 10.0.29 OData ($metadata) url, @gregwinterstein 's fix has generated code that compiles correctly.

Note: by unpatched I mean that our Dynamics 365 has no hotfix for LCS 753226, nor it can be applied - if you have 10.0.30 or 10.0.31 you can apply it, while 10.0.32+ is supposed to incorporate the fix.

[SomeTroglodyte]

@gregwinterstein 's fix has generated code

I'd like to test with non-dynamics sources but am too lazy to compile from source - care to share a build?

[mvarie]

@SomeTroglodyte binaries here: https://www.dropbox.com/sh/n0qxpvijm3all52/AAAg-5UQ1WBT8J4rKj3Msz6ca?dl=0

[SomeTroglodyte]

Took me a while, but I have just now built @gregwinterstein 's "all-fixes" branch from source and I can confirm it generates a working connected service where the current public version fails miserably. A source feed with dashes in field names no longer lets the compiler stumble over all the invalid subtractions...

Sample excerpt

        /// <summary>
        /// There are no comments for Property _SMC_K in the schema.
        /// </summary>
        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.OData.Client.Design.T4", "#VersionNumber#")]
        [global::Microsoft.OData.Client.OriginalNameAttribute("_SMC-K")]
        public virtual string _SMC_K
        {
            get
            {
                return this.__SMC_K;
            }
            set
            {
                this.On_SMC_KChanging(value);
                this.__SMC_K = value;
                this.On_SMC_KChanged();
                this.OnPropertyChanged("_SMC-K");
            }
        }
        [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.OData.Client.Design.T4", "#VersionNumber#")]
        private string __SMC_K;
        partial void On_SMC_KChanging(string value);
        partial void On_SMC_KChanged();

👍 👍 👍

[SomeTroglodyte]

Tested and found working perfectly

[jackypowell]

My team really needs these changes. Can we please get them reviewed soon? Please and thank you! @ElizabethOkerio @KenitoInc @habbes @gathogojr

[habbes]

@gathogojr you had some thoughts around this issue. Could you check this PR?

[gathogojr]

Use Dictionary<TKey,TValue>.TryGetValue(TKey, TValue) method instead of ContainsKey method + Dictionary<TKey,TValue>.Item[TKey] property. TryGetValue should be more efficient and cleaner

[gregwinterstein]

Changed to TryGetValue() in a869504

[gathogojr]

Thank you @gregwinterstein for your contribution.
Have you considered that this could be a breaking change for someone who's REGENERATING the proxy classes?
Specifically for the case of the property named Context.

However, it might also not be a breaking change since the proxy classes that are generated in such a scenario do not compile meaning the affected customers are likely to be manually editing the proxy classes to mitigate the issue.
Either way I think it'd be prudent to try and replicate the scenario by first generating the proxy using the extension before the fix and then regenerating it after the fix.

[SomeTroglodyte]

REGENERATING

I confirm: Regenerating is the most frequent use for me. Without this PR, each run requires 1-2 minutes of manual edits afterwards, per feed. This saves time especially then regenerating.

[gregwinterstein]

Have you considered that this could be a breaking change for someone who's REGENERATING the proxy classes? Specifically for the case of the property named Context.

@gathogojr, it has occurred to me that this pull request may cause some breaking changes because of the way valid identifiers are generated vs the way they were generated previously. I don't have a good solution for that.

My main issue is that I am unable to generate proxies that compile for either Dynamics F&O or Dynamics CRM (this doesn't entirely resolve all of the CRM issues due to inconsistent casing in the CRM metadata). This change generates proxies that compile for Dynmaics F&O and gets a lot closer for Dynamics CRM.

If backward compatibility is an issue, I'm open to suggestions.

[gathogojr]

Thank you @gregwinterstein for your contribution

[SomeTroglodyte]

It might be nice to have some indication here on when this change might become available all the way through normal Visual Studio updates. 2025? 2026?

Edit: This is already available through VS marketplace in 1.1.0:

ChangeLog:
1.1.0
Fix bug affecting excluding of schema types, operations, and operation imports.
Support omitting runtime version and code generation timestamp from generated files.
Fix bug affecting emitting of dynamic properties container property.
Support arm64 product architecture.
Support generation of valid property identifiers when service metadata does not strictly adhere to OData specification on property name.

I just got thrown off because that is not mirrored here on the releases page.