Populating classes with non-settable collections

[nzbart]

I am sure that someone else must have asked this in the past, but I can't find this exact question or a satisfactory answer for it anywhere. There are similar questions that touch on the subject, but don't address it directly.

Microsoft mandates that collection properties must not be settable, I'm not aware of any class in the BCL that violates this. However, when I try to follow this rule, I notice that AutoFixture does not populate the collection. System.Xml.Serialization, JSON.NET, and other tools do respect this rule by calling the Add method on the collection.

Below are two tests that illustrate the issue. The test on the settable list passes, but the test on the non-settable collection fails. I think both should pass.

    class IncorrectClass
    {
        //not valid according to Microsoft guidelines
        public List<int> Integers { get; set; }
    }

    //This test passes
    [Test]
    public void Given_a_class_with_settable_list_property_When_populated_via_autofixture_Then_the_list_should_have_some_elements()
    {
        var instance = new Fixture().Create<IncorrectClass>();

        Assert.That(instance.Integers.Count, Is.Not.EqualTo(0));
    }

    class CorrectClass
    {
        //compliant with Microsoft guidelines at https://msdn.microsoft.com/en-us/library/dn169389%28v=vs.110%29.aspx:
        // "X DO NOT provide settable collection properties."
        public List<int> Integers { get; private set; }

        public CorrectClass()
        {
            Integers = new List<int>();
        }
    }

    //This test currently fails, but I would like it to pass
    [Test]
    public void Given_a_class_with_non_settable_list_property_When_populated_via_autofixture_Then_the_list_should_have_some_elements()
    {
        var instance = new Fixture().Create<CorrectClass>();

        Assert.That(instance.Integers.Count, Is.Not.EqualTo(0));
    }

Is there some way that I can extend or configure AutoFixture to populate the compliant class?

Update:
May be related to https://autofixture.codeplex.com/workitem/4199 and https://stackoverflow.com/questions/5500654/how-to-create-an-ilist-of-anonymous-classes-using-autofixture. However, the tests above are MWEs - I want to be able to run this over many types that are only known at runtime without specifying individual property overrides.

[ploeh]

Believe it or not, but this isn't a question we get particularly often. It's by design, although you could argue that the design is stupid.

On a certain level, it's very consistent with what AutoFixture does in general: it creates objects, and populates it with data, but it doesn't invoke methods.

When you expose ICollection<T>, you could certainly argue that it's data, but the way you fill it with data is by invoking its Add method - and AutoFixture doesn't invoke methods.

However, that's still a stupid argument, because it does populate writeable properties, and those are actually methods in disguise as well.

This I don't write to defend the current behaviour, but only to provide a vague sketch on the original reasoning behind that design decision.

Perhaps we should change the design.

In any cases, here's one way to fill the instance:

[Fact]
public void OneOfFix()
{
    var fixture = new Fixture();
    var instance = fixture.Create<CorrectClass>();

    fixture.AddManyTo(instance.Integers);

    Assert.NotEqual(0, instance.Integers.Count);
}

Notice the use of the AddManyTo method, to populate the collection.

A slightly more persistent workaround is this:

[Fact]
public void CustomizeClass()
{
    var fixture = new Fixture();
    fixture.Customize<CorrectClass>(
        c => c.Do(cc => fixture.AddManyTo(cc.Integers)));
    var instance = fixture.Create<CorrectClass>();
    Assert.NotEqual(0, instance.Integers.Count);
}

In this case, Customize is used to change the behaviour of fixture for as long as fixture exists (or is changed again).

It wouldn't be inconceivable to write a custom ISpecimenCommand that auto-fills all specimens that implement ICollection<T>. However, IIRC, such a thing currently doesn't exist.

[nzbart]

I have come up with a workaround for now, but would like to solve the issue in the idiomatic AutoFixture way.

Unfortunately, it seems that my naive ISpecimenCommand isn't being called for the collection property - probably because it is read-only. What would be the correct way for me to access the property? Is a Postprocessor useful for this?

---

For anyone else who may have the same issue, this is the workaround I am currently using:

public static object CreateInstance(Fixture fixture, Type type)
{
    var context = new SpecimenContext(fixture);
    var value = context.Resolve(new SeededRequest(type, null));
    PopulateCollectionsOnObject(value, fixture);
    return value;
}

public static void PopulateCollectionsOnObject(object value, Fixture fixture)
{
    var numberOfItemsInEachCollection = fixture.RepeatCount;

    var collections = value.GetType().GetProperties().Where(p => typeof(IList).IsAssignableFrom(p.PropertyType) && p.PropertyType.IsGenericType);
    foreach(var collectionProperty in collections)
    {
        var collection = (IList)collectionProperty.GetGetMethod().Invoke(value, null);
        var itemType = collectionProperty.PropertyType.GetGenericArguments().Single();

        for(int i = 0; i != numberOfItemsInEachCollection; ++i)
            collection.Add(CreateInstance(fixture, itemType));
    }
}

[ploeh]

Shouldn't you be using typeof(IList<>) instead of typeof(IList)?

[moodmosaic]

FWIW, here's another approach:

[Fact]
public void CustomizeClass()
{
    var fixture = new Fixture()
        .Customize(new CorrectClassCustomization());
    var instance = fixture.Create<CorrectClass>();
    Assert.NotEmpty(instance.Integers);
}

Where CorrectClassCustomization is defined as:

internal class CorrectClassCustomization : ICustomization
{
    public void Customize(IFixture fixture)
    {
        fixture.Customizations.Add(
            new FilteringSpecimenBuilder(
                new Postprocessor(
                    new MethodInvoker(
                        new ModestConstructorQuery()),
                    new CorrectClassFiller()),
                new CorrectClassSpecification()));
    }

    private class CorrectClassFiller : ISpecimenCommand
    {
        public void Execute(object specimen, ISpecimenContext context)
        {
            if (specimen == null)
                throw new ArgumentNullException("specimen");
            if (context == null)
                throw new ArgumentNullException("context");

            var target = specimen as CorrectClass;
            if (target == null)
                throw new ArgumentException(
                    "The specimen must be an instance of CorrectClass.",
                    "specimen");

            var items =
                (IEnumerable<int>)context.Resolve(typeof(IEnumerable<int>));
            target.Integers.AddRange(items);
        }
    }

    private class CorrectClassSpecification : IRequestSpecification
    {
        public bool IsSatisfiedBy(object request)
        {
            var requestType = request as Type;
            if (requestType == null)
                return false;
            return typeof(CorrectClass).IsAssignableFrom(requestType);
        }
    }
}

[ploeh]

@moodmosaic, @adamchester, @ecampidoglio, should we add default behaviour to Fixture so that it fills ICollection<T>? Would it be a breaking change?

[moodmosaic]

This shouldn't be a breaking change. – (Maybe, that's how it should probably work from the beginning of v3, since various collections are filled by default anyway.)

---

If I understand this correctly, based on the above discussion, we'd like the following test to pass

[Fact]
public void Foo()
{
    var fixture = new Fixture();
    var actual = fixture.Create<MyClass>();
    Assert.NotEmpty(actual.SomeCollection);
}

by adding default behaviour to Fixture so that it invokes

ICollection<T>.Add(T item)

where MyClass is defined as

public class MyClass
{
    public MyClass()
    {
        SomeCollection = new List<int>();
    }

    public List<int> SomeCollection { get; private set; }
}

[ploeh]

I've tentatively changed the status of this issue to a potential new feature.

If this feature can be added without breaking any existing tests, I think we can assume that it's not a breaking change.

If tests start failing left and right, we'd have to postpone this to AutoFixture 4.

[moodmosaic]

👍

[MendelMonteiro]

I'd definitely make use of this feature and I was also as confused when it didn't just work out of the box.

[Loppor]

If I understand this issue correctly: We need to create an extra generator to handle the population of a generic list, right? Just to get some general sense of direction of where to search 😉

[Loppor]

@zvirja Could you perhaps shed some light on this situation as well?

[kentcb]

I know this is an old issue, but I've been trying to achieve the same today because I'm trying to auto-populate some protobuf objects, and collections are auto-generated as get-only instances of RepeatedField<T>.

I can get it to work for specific properties on a specific type, but I'm struggling to find a generalized solution that will work across all my protobufs without case-by-case intervention.

The problem is that if I return true from IRequestSpecification.IsSatisfiedBy, then I am expected to populate the entire object rather than just its get-only collection properties. And I haven't found a simple way to do that yet.

And if I return false from IRequestSpecification.IsSatisfiedBy for the containing type, then I'm never given the opportunity to return true for any of its get-only properties.

Does anyone have any thoughts on this?

[kentcb]

Here's some "working" code I arrived at. It's ugly, I know.

    // Populate get-only collections, such as Protobuf repeated fields
    internal sealed class GetOnlyCollectionsCustomization : ICustomization
    {
        public void Customize(IFixture fixture)
        {
            fixture.Customizations.Add(
                new FilteringSpecimenBuilder(
                    new Postprocessor(
                        new MethodInvoker(
                            new ModestConstructorQuery()),
                            new GetOnlyCollectionsCustomizationFiller()),
                    new GetOnlyCollectionsSpecification()));
        }

        private sealed class GetOnlyCollectionsCustomizationFiller : ISpecimenCommand
        {
            public void Execute(object specimen, ISpecimenContext context)
            {
                Ensure.ArgumentNotNull(specimen, nameof(specimen));
                Ensure.ArgumentNotNull(context, nameof(context));

                var allProperties = specimen
                    .GetType()
                    .GetProperties(BindingFlags.Public | BindingFlags.Instance | BindingFlags.GetProperty)
                    .ToList();

                var getOnlyCollectionProperties = allProperties
                    .Where(property => !property.CanWrite)
                    .Where(property => property.PropertyType.IsGenericType && property.PropertyType.GenericTypeArguments.Length == 1)
                    .Where(property => typeof(IList).IsAssignableFrom(property.PropertyType))
                    .ToList();

                var nonCollectionProperties = allProperties
                    .Where(property => property.CanWrite)
                    .Except(getOnlyCollectionProperties)
                    .ToList();

                foreach (var getOnlyCollectionProperty in getOnlyCollectionProperties)
                {
                    var list = (IList)getOnlyCollectionProperty.GetValue(specimen);

                    if (list == null)
                    {
                        continue;
                    }

                    var enumerableType = typeof(IEnumerable<>).MakeGenericType(getOnlyCollectionProperty.PropertyType.GenericTypeArguments[0]);
                    var items = (IEnumerable)context.Resolve(enumerableType);

                    foreach (var item in items)
                    {
                        list.Add(item);
                    }
                }

                // Fill in the remaining properties.
                foreach (var nonCollectionProperty in nonCollectionProperties)
                {
                    var value = context.Resolve(nonCollectionProperty.PropertyType);
                    nonCollectionProperty.SetValue(specimen, value);
                }
            }
        }

        private sealed class GetOnlyCollectionsSpecification : IRequestSpecification
        {
            public bool IsSatisfiedBy(object request)
            {
                var requestType = request as Type;

                if (requestType == null)
                {
                    return false;
                }

                var getOnlyCollectionProperties = requestType
                    .GetProperties(BindingFlags.Public | BindingFlags.Instance | BindingFlags.GetProperty)
                    .Where(property => !property.CanWrite)
                    .Where(property => property.PropertyType.IsGenericType && property.PropertyType.GenericTypeArguments.Length == 1)
                    .Where(property => typeof(IList).IsAssignableFrom(property.PropertyType))
                    .ToList();

                return getOnlyCollectionProperties.Count > 0;
            }
        }
    }

I suspect there's probably a better way to achieve this. I'd love to hear any ideas.

[zvirja]

@kentcb Thanks for sharing the sample!

The code itself looks reasonable, however it affects the object activation. Now all the objects which match the criteria are activated by the ModestConstructorQuery, which omits some other ways the core has to activate objects.

Instead, I would suggest you to write a behavior (see fixture.Behaviors). This way you could intercept the already activated objects and just fill the read-only properties 😉 It should be less invasive. But anyway, you will pay the significant const of type inspection on each request 😟

[kentcb]

@zvirja I see your point regarding object activation. However, I'm having a hard time getting my head around how to write a behavior. The only docs I can find are these, which aren't helpful in understanding the process of writing a behavior. And all the examples I can find online are specifically about recursion, which is not the problem here.

I'm trying to stumble my way through it by just writing code and seeing what happens, but even with a bare outline of code I'm getting:

System.InvalidOperationExceptions: Unable to find node matching the specified predicate.

Do you know of any good examples/docs on writing behaviors for AutoFixture?

[vivainio]

I don't know where this is going currently, but AutoFixture not supporting Protobuf RepeatedField "out of the box" is a drag, and protobuf design is unlikely to change

[charles-salmon]

@vivainio I'm currently working on this and will have a PR up within the next few days. It doesn't look like anything has been merged in since last July, so hopefully someone will be available to review these changes.