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

DataGrid "Cannot create an abstract class" when UseValidation=true #3318

Closed
WolfgangKluge opened this issue Jan 13, 2022 · 3 comments
Closed
Assignees
Labels
Type: Bug 🐞 Something isn't working

Comments

@WolfgangKluge
Copy link
Contributor

Describe the bug
If I try to add or edit a row with a specific type (contains an ICollection<>-Property), I get the above error message. The stack trace is

System.MemberAccessException: Cannot create an abstract class.
   at System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObjectInternal(Type type)
   at System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(Type type)
   at Blazorise.DataGrid.Utils.RecursiveObjectActivator.CreateInstanceRecursive(Object currObjInstance, PropertyInfo[] properties, ValueTuple`2 typeTracker, Int32 depthLevel, Int32 circularReferenceLevel)
   at Blazorise.DataGrid.Utils.RecursiveObjectActivator.CreateInstanceRecursive(Object currObjInstance, PropertyInfo[] properties, ValueTuple`2 typeTracker, Int32 depthLevel, Int32 circularReferenceLevel)
   at Blazorise.DataGrid.Utils.RecursiveObjectActivator.CreateInstance[TItem]()
   at Blazorise.DataGrid.DataGrid`1.InitEditItem(TItem item)
   at Blazorise.DataGrid.DataGrid`1.Edit(TItem item)
   at Blazorise.DataGrid._BaseDataGridRowCommand`1.<get_Edit>b__6_0()

To Reproduce
Here's an example

@code{
    class Test {
        public int Num { get; set; }

        //public List<string>? Values { get; set; } // works
        public IList<string>? Values { get; set; } // should work
        //public IList<string> Values { get; set; } = new List<string>(); // should work
    }

    List<Test> Data = new List<Test> {
        new() { Num = 1 },
        new() { Num = 2 },
        new() { Num = 3, Values = new List<string>() }
    };
}

<DataGrid
    TItem="Test"
    Data="@Data"
    Editable
    UseValidation
>
    <DataGridColumns>
        <DataGridCommandColumn TItem="Test" />
        <DataGridColumn TItem="Test" Field="Num" Editable />
        <DataGridColumn TItem="Test">
            <DisplayTemplate>@(context.Values == null)</DisplayTemplate>
        </DataGridColumn>
    </DataGridColumns>
</DataGrid>
  1. Click New or Edit on any entry and see the error
  2. a) Even on an object where Values is already set (use of property initializer or Num = 3) no editing is available
    b) If you use List<> instead of IList<> it works.

Expected behavior
Ideally, there's no difference between List<> and IList<>-properties.
Since that will mean that you have to guess what implementation of an interface one will have to use, I think at least the property should be empty (and initializers should not be overwritten).
So in that cases empty (without error message) would/should be fine.

Additional context
While investigating, I've tested IsListOrCollection, too (don't know if that's part of the problem/soluton). And that behaves a bit strange to me (but I'm unsure)

typeof(List<string>).IsListOrCollection() // true
typeof(string[]).IsListOrCollection() // true
typeof(IList<string>).IsListOrCollection() // false
typeof(IEnumerable<string>).IsListOrCollection() // false
typeof(ICollection<string>).IsListOrCollection() // false

The current code is

public static bool IsListOrCollection( this Type type )
=> typeof( System.Collections.IList ).IsAssignableFrom( type )
|| typeof( System.Collections.ICollection ).IsAssignableFrom( type )
|| typeof( IEnumerable<> ).IsAssignableFrom( type );

Since IList inherits ICollection, the first line is superflous. But the striking part is the last line. I've never seen open generics used in that way and I think (and have tested) that this returns (nearly) always false.

typeof(IEnumerable<>).IsAssignableFrom(typeof(List<string>)) // false
typeof(IEnumerable<>).IsAssignableFrom(typeof(string[])) // false
typeof(IEnumerable<>).IsAssignableFrom(typeof(IList<string>)) // false
typeof(IEnumerable<>).IsAssignableFrom(typeof(IEnumerable<string>)) // false
typeof(IEnumerable<>).IsAssignableFrom(typeof(ICollection<string>)) // false
typeof(IEnumerable<>).IsAssignableFrom(typeof(ICollection<>)); // false
typeof(IEnumerable<>).IsAssignableFrom(typeof(IEnumerable<>)); // true

If you want to detect all enumerables, just use typeof(IEnumerable).IsAssignableFrom(type) (IEnumerable<> inherits IEnumerable). But by doing so, you will have all the collection types returning true, thus e.g. typeof(string) (as it's a collection of char), too.. Don't know if that's the desired behavior.
From the name IsListOrCollection I assume you want to detect collections only, so this might be a solution

public static bool IsListOrCollection( this Type type )
    => typeof(ICollection).IsAssignableFrom( type )
    || type.IsGenericCollection()
    || Array.Find( type.GetInterfaces(), IsGenericCollection ) != null;

private static bool IsGenericCollection( this Type type )
    => type.IsGenericType
    && type.GetGenericTypeDefinition() == typeof( ICollection<> );

(should be renamed to IsCollection..)

@WolfgangKluge WolfgangKluge added the Type: Bug 🐞 Something isn't working label Jan 13, 2022
@David-Moreira
Copy link
Contributor

David-Moreira commented Jan 31, 2022

Hello,

So the problem is that when you go into edit mode, internally we have to create a dummy object based on your class for validation purposes. Since you've provided an interface it does not know how to instantiate it, therefore throws the error System.MemberAccessException: Cannot create an abstract class.

So my guess is that we can ignore interfaces as that can be virtually anything and we can't know how to provide an instance for it, it should also have no negative effect on the DataGrid's validation I believe, right @stsrki ?
Another thing we could consider is that when instantiating the dummy object internally, only consider the actual Column Field's that have been provided.

Edit:

if ( !currType.IsValueType && currType != typeof( string ) && !currType.IsListOrCollection() )
    then tries to instantiate

You are also right, that if IsListOrCollection had detected the IList as a list/collection it would not have thrown the error as it would have not tried to instantiate it. So that's something we'll be looking at, and actually set up some unit tests to validate the use cases like you've done on your message, thanks for noticing.

@stsrki
Copy link
Collaborator

stsrki commented Jan 31, 2022

Can we also do something similar to what we have for the new item? We have NewItemCreator. So maybe we can also have ValidationItemCreator or EditItemCreator that allows the user to manually create an object. Once create we just proceed with assigning the internal values as usual.

@David-Moreira
Copy link
Contributor

David-Moreira commented Feb 1, 2022

Seems like an idea. So fix IsListOrCollection on 0.9.5 which should fix the OP's issue.
Introduce ValidationItemCreator or EditItemCreator on 1.0.0. I like EditItemCreator. It's closer to the already existing NewItemCreator and at the moment we use the creator whenver there's validation or not, so....

This was referenced Feb 1, 2022
@stsrki stsrki added this to the 0.9.5 Support milestone Feb 6, 2022
@stsrki stsrki closed this as completed Feb 6, 2022
@stsrki stsrki added this to Support Aug 3, 2024
@stsrki stsrki moved this to ✔ Done in Support Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐞 Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants