Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple issues with generated C# code #1101

Closed
wvdvegt opened this issue Dec 13, 2017 · 9 comments
Closed

Multiple issues with generated C# code #1101

wvdvegt opened this issue Dec 13, 2017 · 9 comments

Comments

@wvdvegt
Copy link

wvdvegt commented Dec 13, 2017

Hi

Swagger definition used https://esi.tech.ccp.is/latest/swagger.json (this is a nice & complex one).

It seems to generate 'close' to working code only with SingleClientFroIOperationId as operation mode.

But still some major issues left (for the rest great work that saves a lotta work).

  1. Some of the enums (Color/LabelColor) use names during serialization like #0000fe which are illegal in C#. So better replace these illegal chars by an _.

  2. Quite some enums value (like Language in this example) differ in name from the EnumMember value. When passed into a method they are added to the url/body as text using an ToString() like:

if (language != null) urlBuilder_.Append("language=").Append(Uri.EscapeDataString(System.Convert.ToString(**language**, CultureInfo.InvariantCulture))).Append("&");

This results in for example "EnUs" being appended instead of the EnumMember value of the language "en-us".

Instead of the enum value, reflection should be used to retrieve the correct value to be appended.

  1. It would be nice (for larger definitions like this one) if there was an option to split the current partial client class on the first level of path (so get_alliances/put_alliances etc methods would go into a separate file)

  2. The trailing / of paths is lost in the url's used in code. See for example the path
    "/characters/{character_id}/assets/names/"
    which should results in
    "/characters/{character_id}/assets/names/?"
    inside
    Post_characters_character_id_assets_namesAsync()
    but is emitted as
    "/characters/{character_id}/assets/names?"
    which fails when calling the webservice.

@wvdvegt
Copy link
Author

wvdvegt commented Dec 13, 2017

Possible solution for issue 2

Using the extension methods

namespace EveEsi
{
    using System;
    using System.Globalization;
    using System.Reflection;
    using System.Runtime.Serialization;

    /// <summary>
    /// A convert extension.
    /// </summary>
    public static class ConvertExtension
    {
        /// <summary>
        /// An Object extension method that gets a description.
        /// </summary>
        ///
        /// <param name="value"> The value to act on. </param>
        ///
        /// <returns>
        /// The description.
        /// </returns>
        public static string GetDescription(this Object value)
        {
            return value.GetDescription(CultureInfo.InvariantCulture);
        }

        /// <summary>
        /// An Object extension method that gets a description.
        /// </summary>
        ///
        /// <param name="value"> The value to act on. </param>
        /// <param name="ci">    The ci. </param>
        ///
        /// <returns>
        /// The description.
        /// </returns>
        public static string GetDescription(this Object value, CultureInfo ci)
        {
            if (value is Enum)
            {
                string name = Enum.GetName(value.GetType(), value);
                if (name != null)
                {
                    FieldInfo field = value.GetType().GetField(name);
                    if (field != null)
                    {
                        EnumMemberAttribute attr = Attribute.GetCustomAttribute(field, typeof(EnumMemberAttribute)) as EnumMemberAttribute;
                        if (attr != null)
                        {
                            return attr.Value;
                        }
                    }
                }
            }

            return System.Convert.ToString(value, ci);
        }
    }
}

It's possible to rewrite generated code like:
System.Convert.ToString(language, CultureInfo.InvariantCulture)
into
datasource.GetDescription(CultureInfo.InvariantCulture)
which retrieves the EnumMember Attribute's value (of if missing or another type then an enum is passed, defaults to
System,Convert.ToString()

Tested on the code that was generated it now creates a correct webservice call url.

@RicoSuter
Copy link
Owner

  1. must be fixed in https://github.com/RSuter/NJsonSchema/blob/master/src/NJsonSchema.CodeGeneration/DefaultEnumNameGenerator.cs
  2. Duplicate of: Enumeration values don't respect original letter casing in generated C# clients when used in URL query #1052
  3. Similar to Epic: Multiple file output NJsonSchema#514 (not yet completed in NSwag)
  4. Why is your API not accepting /characters/{character_id}/assets/names?? If this is a problem for some APIs we should not trim /

@wvdvegt
Copy link
Author

wvdvegt commented Dec 13, 2017

  1. Minus signs are removed for example, so better use the attribute value (it contains the correct value).
    Changing the template used for generating code and use code like i suggested might work better.
  2. Issue is broader then just case issues (so not a dupe of Enumeration values don't respect original letter casing in generated C# clients when used in URL query #1052).
  3. It just doesn't and API/Definition is not mine.

RicoSuter added a commit to RicoSuter/NJsonSchema that referenced this issue Dec 13, 2017
@RicoSuter
Copy link
Owner

What if we generate the following method in each client class:

private string ConvertToString(object value, System.Globalization.CultureInfo cultureInfo)
{
    if (value is System.Enum)
    {
        string name = System.Enum.GetName(value.GetType(), value);
        if (name != null)
        {
            var field = System.Reflection.IntrospectionExtensions.GetTypeInfo(value.GetType()).GetDeclaredField(name);
            if (field != null)
            {
                var attribute = System.Reflection.CustomAttributeExtensions.GetCustomAttribute(field, typeof(System.Runtime.Serialization.EnumMemberAttribute)) 
                    as System.Runtime.Serialization.EnumMemberAttribute;
                if (attribute != null)
                {
                    return attribute.Value;
                }
            }
        }
    }

    return System.Convert.ToString(value, cultureInfo);
}

and use this instead of System.Convert.ToString

@wvdvegt
Copy link
Author

wvdvegt commented Dec 13, 2017

Override the enum's ToString() is probably a better spot for the code.

@RicoSuter
Copy link
Owner

How can you override ToString() of an enum?

@wvdvegt
Copy link
Author

wvdvegt commented Dec 13, 2017

Hi

You're right about ToString(), but the ToString(CultureInfo ci) you can catch with an extension methods.

Your solution also seems pretty ok to me.

Btw nice work NSwagStudio.

You only have to change the
System.Convert.ToString(datasource, CultureInfo.InvariantCulture)
to
datasource.ToString(CultureInfo.InvariantCulture)

namespace EveEsi
{
    using System;
    using System.Globalization;
    using System.Reflection;
    using System.Runtime.Serialization;

    /// <summary>
    /// A convert extension.
    /// </summary>
    public static class ConvertExtension
    {
        /// <summary>
        /// An Object extension method that gets a description.
        /// </summary>
        ///
        /// <param name="value"> The value to act on. </param>
        /// <param name="ci">    The ci. </param>
        ///
        /// <returns>
        /// The description.
        /// </returns>
        public static string ToString(this Enum value, CultureInfo ci)
        {
            if (value is Enum)
            {
                string name = Enum.GetName(value.GetType(), value);
                if (name != null)
                {
                    FieldInfo field = value.GetType().GetField(name);
                    if (field != null)
                    {
                        EnumMemberAttribute attr = Attribute.GetCustomAttribute(field, typeof(EnumMemberAttribute)) as EnumMemberAttribute;
                        if (attr != null)
                        {
                            return attr.Value;
                        }
                    }
                }
            }

            return System.Convert.ToString(value, ci);
        }
    }
}

Using generics also works if you define the method like:

       public static string ToString<T>(this T value, CultureInfo ci) where T : struct
        {
        if (typeof(T).IsENum()) {
                        //
                }
        }

@john-e-lewis
Copy link

Just come across this enum value translation issue on a different swagger definition.
Using the ConvertToString method in the client instead of System.Convert.ToString resolved the issue.

@RicoSuter
Copy link
Owner

Please retest with latest CI build, and open new separate issues if needed..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants