ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.23k stars 745 forks source link

Enumeration Classes error out #5251

Open PandaBlackAndWhitr opened 2 years ago

PandaBlackAndWhitr commented 2 years ago

Is there an existing issue for this?

Describe the bug

When using Enumeration classes, attempts to bind the classes results in an error.

Given an EnumType definition for enum class AccountType:

public class AccountTypeEnumType: EnumType<AccountType>
{
  protected override void Configure(IEnumTypeDescriptor<AccountType> descriptor)
  {
    descriptor.Name(typeof(AccountType).Name);

    descriptor.BindValuesExplicitly();

    foreach(var enumeration in Enumeration.GetAll<AccountType>())
    {
      descriptor.Value(enumeration).Name(enumeration.Name);
    }
  }
}

Expected:

Actual: For a class Fufu.Domain.Aggregates.AccountAggregate.AccountType: the following error results:

An unhandled exception has occurred while executing the request.
HotChocolate.SchemaException: For more details look at the `Errors` property.

1. `FUFU._DOMAIN._AGGREGATES._ACCOUNT_AGGREGATE._ACCOUNT_TYPE` is not a valid GraphQL type name. (Parameter 'value') (Fufu.API.GraphQL.Objects.AccountTypeEnumType)

   at HotChocolate.Configuration.TypeInitializer.DiscoverTypes()
   at HotChocolate.Configuration.TypeInitializer.Initialize()
   at HotChocolate.SchemaBuilder.Setup.InitializeTypes(SchemaBuilder builder, IDescriptorContext context, IReadOnlyList`1 types, LazySchema lazySchema)
   at HotChocolate.SchemaBuilder.Setup.Create(SchemaBuilder builder, LazySchema lazySchema, IDescriptorContext context)

The error seems to occur during naming:

   at HotChocolate.NameString..ctor(String value)
   at HotChocolate.NameString.ConvertFromString(String s)
   at HotChocolate.Types.Descriptors.DefaultNamingConventions.GetEnumValueName(Object value)

Steps to reproduce

  1. Create an Enumeration base class using the sample Microsoft enumeration classes, and extend with any arbitrary enum
  2. Define the enumeration according to HotChocolate documentation, setting values to enumeration classes and names to Enumeration.Name (see bug description for exact values)
  3. Attempt to generate the schema

Relevant log output

Exception has occurred: CLR/System.ArgumentException
An exception of type 'System.ArgumentException' occurred in HotChocolate.Abstractions.dll but was not handled in user code: '`FUFU._DOMAIN._AGGREGATES._ACCOUNT_AGGREGATE._ACCOUNT_TYPE` is not a valid GraphQL type name.'
   at HotChocolate.NameString..ctor(String value)
   at HotChocolate.NameString.ConvertFromString(String s)
   at HotChocolate.Types.Descriptors.DefaultNamingConventions.GetEnumValueName(Object value)
   at HotChocolate.Types.Descriptors.EnumValueDescriptor..ctor(IDescriptorContext context, Object runtimeValue)
   at HotChocolate.Types.Descriptors.EnumValueDescriptor.New(IDescriptorContext context, Object value)
   at HotChocolate.Types.Descriptors.EnumTypeDescriptor.Value[T](T value)
   at HotChocolate.Types.Descriptors.EnumTypeDescriptor`1.Value(T value)
   at Fufu.API.GraphQL.Objects.AccountTypeEnumType.Configure(IEnumTypeDescriptor`1 descriptor) in /home/travisdev/Documents/financeServer/fufu/Fufu/Fufu.API/GraphQL/Objects/AccountAggregate/AccountTypeEnum.cs:line 42
   at HotChocolate.Types.EnumType`1.CreateDefinition(ITypeDiscoveryContext context)
   at HotChocolate.Types.TypeSystemObjectBase`1.Initialize(ITypeDiscoveryContext context)
   at HotChocolate.Configuration.TypeRegistrar.InitializeType(TypeSystemObjectBase typeSystemObject, String scope, Boolean isInferred)

Additional Context?

Note that a workaround to the issue is defining custom NamingConventions:

class NamingConventions: DefaultNamingConventions
{
  public override NameString GetEnumValueName(object value)
  {
    if (value is Enumeration enumeration)
    {
      return base.GetEnumValueName(enumeration.Name);
    }

    return base.GetEnumValueName(value);
  }
}

Which implies the issue being in this/around this GetEnumValueName call.

I feel like everything I do concerning HotChocolate entails workarounds though...

Product

Hot Chocolate

Version

12.11.0

michaelstaib commented 2 years ago

The issue lies somehow in the naming convention. You could override the default naming convention if you want to do a quick fix in your project, or override the ToString method of the enumeration.

I will create a test and we can fix that with the next release.

jimitndiaye commented 2 years ago

I'm having this same issue with both scalar types (int) and classes like in the OP.

williamdenton commented 8 months ago

Ive just run into the same/similar issue

a smart enum class like this (note: ToString() will return a non graphql safe name)

public sealed class CultureCode
{
    private readonly string _culture;

    public static CultureCode EN_GB { get; } = new("en-GB";);

    public static CultureCode EN_NZ { get; } = new("en-NZ";);

    private CultureCode(string culture)
    {
        _culture = culture;
    }

    public override string ToString()
        => _culture;
}

and an enum descriptor like this

protected override void Configure(IEnumTypeDescriptor<CultureCode> descriptor)
{
    descriptor
        .Value(CultureCode.EN_GB) // will use ToString() to get the name and throw exception as "-" is invalid
        .Name(CultureCode.EN_GB.ToString().Replace("-", "_")) // wont run as the exception is thrown earlier in the fluent api
        .Description(CultureCode.EN_GB.ToString());
}

My fix is to override the DefaultNamingConventions like this

        public override string GetEnumValueName(object value)
        {
            var basename = base.GetEnumValueName(value);

            if (basename.Contains('-'))
                basename = basename.Replace('-', '_');

            return basename;
        }

but i think the fluent api would use a new overload for .Value()

https://github.com/ChilliCream/graphql-platform/blob/09eb3c08d364f3749d0e82333abb2409027e57c6/src/HotChocolate/Core/src/Types/Types/Descriptors/Contracts/IEnumTypeDescriptor~1.cs#L39

IEnumValueDescriptor Value(TRuntimeType value);
IEnumValueDescriptor Value(TRuntimeType value, string enumValueName);

https://github.com/ChilliCream/graphql-platform/blob/09eb3c08d364f3749d0e82333abb2409027e57c6/src/HotChocolate/Core/src/Types/Types/Descriptors/EnumTypeDescriptor.cs#L124-L138

which would plumb the name through to

        descriptor = EnumValueDescriptor.New(Context, value);
        descriptor.Name(enumValueName);

though even this may be too late, it may need to be passed to .New()