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.27k stars 748 forks source link

Support skipping readonly fields #6128

Closed xionglingfeng closed 1 year ago

xionglingfeng commented 1 year ago

Product

Hot Chocolate

Is your feature request related to a problem?

We have a class generated from XSD. This class include generated fields like XXSpecified, which are mandatory to XmlSerializer. For example:

/// <summary>
/// <para xml:lang="en">Gets a value indicating whether the Error collection is empty.</para>
/// </summary>
[XmlIgnoreAttribute()]
public bool ErrorSpecified
{
    get
    {
        return (this.Error.Count != 0);
    }
}

Once we use this class as an Input type, HotChocolate will complain that the XXSpecified fields are read-only. Can we have options to allow skipping read-only fields?

The solution you'd like

Read-only fields are automatically skipped when using as Input.

michaelstaib commented 1 year ago

Can you create a small repro showing the issue?

xionglingfeng commented 1 year ago

The following code should demonstrate the issue:

public class MyInputClass
{
    private List<string> _names;

    public MyInputClass()
    {
        this._names = new List<string>();
    }

    public List<string> Names
    {
        get
        {
            return this._names;
        }
    }
}

public class Query
{
    public bool Test(MyInputClass input)
    {
        return true;
    }
}

The Test method use MyInputClass as Input. HotChocolate will try to generate a schema for MyInputClass. However, it complains that there is a property (Names) is read-only and cannot be set, then refuse to generate schema.

The current behaviour cause trouble when using together with XmlSerializer (i.e., directly use a XmlSerializer-compatiable class as GraphQL schema) due to a field (XXSpecified) is required by XmlSerializer and it is read-only.

I think an expected behaviour is that HotChocolate should continue generating schema and ignoring the read-only property.

senecaconsultancy commented 1 year ago

perhaps better than skipping the field, in the general case, would be to treat it as readonly, allowing its use in right-hand side (rvalue?) expressions.

xionglingfeng commented 1 year ago

Yes. I would expect HotChocolate to skip this field when generating schema for MyInputClassInput. However, the field can be kept when generating schema for MyInputClass (not Input).

This read-only field is actually expected when using the class as a return type. Thus I cannot apply [GraphQLIgnore].

michaelstaib commented 1 year ago

In this case the GraphQL schema would be invalid as the input would end up with no fields which is not allowed by the GraphQL spec. What you can do is specify type configurations that describe the input and output behavior.

services
    .AddGraphQLServer()
    .AddInputType<MyInputClass>(t => t.Ignore(d => d.Names));

If you have multiple of these you can override the DefaultTypeInspector and ignore these on input. But as I said, it will fail if the input ha no other fields.

xionglingfeng commented 1 year ago

@michaelstaib the read-only fields are everywhere, since the class is generated. Thus, it is very difficult/impractical to skip them one by one. I think it would be great if HotChocolate can provide an feature (e.g., a switch) to ignore 1. object without any fields; 2. read-only fields for input objects.

michaelstaib commented 1 year ago

This is far more complex than you think ... the schema building has to remove all resolvers that have this particular type than as an argument which again could lead to object types without fields.

For now we have no plan to do that. If you want to do that for your solution you can by using a typeinterceptor ... in there you can hook into the schema building and rewrite types.

michaelstaib commented 1 year ago

Closing this issue as we do not plan any work on this for now.

michaelstaib commented 1 year ago

BTW ... a solution here is also to create an internal constructor that allows the serialization to inject the read-only values.

public class MyInputClass
{
    private List<string> _names;

    public MyInputClass()
    {
        this._names = new List<string>();
    }

        internal MyInputClass(List<string> names)
    {
        this._names = names;
    }

    public List<string> Names
    {
        get
        {
            return this._names;
        }
    }
}