Potentially unintended behavior change of circularDependencyBehaviour().omitSpecimen()

[TWiStErRob]

When upgrading from 2.7.0 to 2.7.2 I got some weird failures.

I narrowed it down to V2.7.0...V2.7.1 range and specifically this commit: 4306daa

Consider the following data structure:

class Container {
    public final @NonNull List<Item> items;

    public Container(@NonNull List<Item> items) {
        this.items = items;
//        this.items = Collections.unmodifiableList(items);
    }
}

class Item {
    public final @NonNull List<Ref> refs;

    public Item(@NonNull List<Ref> refs) {
        this.refs = refs;
//        this.refs = Collections.unmodifiableList(refs);
    }
}

class Ref {
    public final @NonNull Container referenced;

    public Ref(@NonNull Container referenced) {
        this.referenced = referenced;
    }
}

and a test:

import androidx.annotation.NonNull;

import com.flextrade.jfixture.JFixture;
import com.flextrade.jfixture.utility.SpecimenType;

import org.junit.Test;

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Properties;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;

public class OmitSpecimenTest {

    @Test
    public void testRecursionContainer() {
        JFixture fixture = new JFixture();
        fixture.customise().circularDependencyBehaviour().omitSpecimen();

        Container container = fixture.create(Container.class);

        assertNotNull(container);
        assertNotNull(container.items);
        assertEquals(3, container.items.size());
        for (Item item : container.items) {
            assertNotNull(item);
            assertNotNull(item.refs);
            assertEquals(3, item.refs.size());
            for (Ref ref : item.refs) {
                assertNotNull(ref);
                assertNull(ref.referenced); // Container recurses
            }
        }
    }

    @Test
    public void testRecursionList() {
        JFixture fixture = new JFixture();
        fixture.customise().circularDependencyBehaviour().omitSpecimen();

        List<Ref> refs = fixture.create(new SpecimenType<List<Ref>>() { });

        assertEquals(3, refs.size());
        for (Ref ref : refs) {
            assertNotNull(ref);
            assertNotNull(ref.referenced);
            assertNotNull(ref.referenced.items);
            assertEquals(3, ref.referenced.items.size());
            for (Item item : ref.referenced.items) {
                assertNotNull(item);
                switch (jfixtureVersion()) {
                    case "2.7.0":
                        assertNotNull(item.refs);
                        assertEquals(0, item.refs.size()); // Ref recurses
                        break;
                    case "2.7.1":
                    case "2.7.2":
                        assertNull(item.refs); // List<Ref> recurses
                        break;
                    default:
                        fail("Unknown version: " + jfixtureVersion());
                }
            }
        }
    }

    private static String jfixtureVersion() {
        InputStream stream = JFixture.class
                .getResourceAsStream("/META-INF/maven/com.flextrade.jfixture/jfixture/pom.properties");
        try {
            Properties properties = new Properties();
            properties.load(stream);
            stream.close();
            return properties.getProperty("version");
        } catch (IOException ex) {
            // ignore; bad pattern for catch and close, but don't care for repro.
            return ex.toString();
        }
    }
}

Running the above testRecursionContainer test runs fine on all versions, but as you can see the expectation has changed in testRecursionList and it behaves differently on different versions: in 2.7.0 the recursive structure ended up being an empty list, but after 4306daa this became a null list. This wouldn't be a problem, but as you can see the commented (this.refs = Collections.unmodifiableList(refs);) line in the model, passing in a null will crash unmodifiableList and that will in turn cause this exception (only on 2.7.1 and 2.7.2):

java.lang.UnsupportedOperationException: JFixture was unable to create an instance of com.example.Item
Most likely because it has no public constructor, is an abstract or non-public type or has no static factory methods.
If this isn't the case it's likely that all constructors and factory methods on the type have thrown an exception.
To view any thrown exceptions just add the Tracing Customisation to the JFixture instance, e.g. fixture.customise(new TracingCustomisation(System.out));

		java.util.List<com.example.Ref> --> 
		com.example.Ref --> 
		public com.example.Ref(com.example.Container) --> 
		com.example.Container --> 
		public com.example.Container(java.util.List) --> 
		java.util.List<com.example.Item> --> 
		com.example.Item --> 
		public com.example.Item(java.util.List) --> 
		com.example.Item

	at com.flextrade.jfixture.behaviours.noresolution.ThrowingNoResolutionHandler.handleNoResolution(ThrowingNoResolutionHandler.java:9)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.handleNoResolutionForRequest(NoResolutionGuard.java:52)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:29)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.builders.MultipleSpecimenRelay.buildArrayList(MultipleSpecimenRelay.java:33)
	at com.flextrade.jfixture.builders.MultipleSpecimenRelay.create(MultipleSpecimenRelay.java:26)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.builders.IterableRelay.create(IterableRelay.java:34)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.utility.ParameterUtils.convertTypesToInstances(ParameterUtils.java:28)
	at com.flextrade.jfixture.utility.ParameterUtils.getConstructorArguments(ParameterUtils.java:16)
	at com.flextrade.jfixture.builders.GenericConstructorRelay.create(GenericConstructorRelay.java:27)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.builders.ClassToConstructorRelay.create(ClassToConstructorRelay.java:51)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.utility.ParameterUtils.convertTypesToInstances(ParameterUtils.java:28)
	at com.flextrade.jfixture.utility.ParameterUtils.getConstructorArguments(ParameterUtils.java:16)
	at com.flextrade.jfixture.builders.GenericConstructorRelay.create(GenericConstructorRelay.java:27)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.builders.ClassToConstructorRelay.create(ClassToConstructorRelay.java:51)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.builders.MultipleSpecimenRelay.buildArrayList(MultipleSpecimenRelay.java:33)
	at com.flextrade.jfixture.builders.MultipleSpecimenRelay.create(MultipleSpecimenRelay.java:26)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.SpecimenBuilderContext.resolve(SpecimenBuilderContext.java:13)
	at com.flextrade.jfixture.builders.IterableRelay.create(IterableRelay.java:34)
	at com.flextrade.jfixture.builders.CompositeBuilder.create(CompositeBuilder.java:26)
	at com.flextrade.jfixture.behaviours.autoproperty.AutoPropertyBuilder.create(AutoPropertyBuilder.java:22)
	at com.flextrade.jfixture.behaviours.noresolution.NoResolutionGuard.create(NoResolutionGuard.java:27)
	at com.flextrade.jfixture.behaviours.specimentype.SpecimenTypeInjector.create(SpecimenTypeInjector.java:26)
	at com.flextrade.jfixture.behaviours.recursion.RecursionGuard.create(RecursionGuard.java:33)
	at com.flextrade.jfixture.JFixture.create(JFixture.java:82)
	at com.flextrade.jfixture.JFixture.create(JFixture.java:73)
	at com.example.OmitSpecimentTest.testRecursionList(OmitSpecimentTest.java:28)

So it looks like the fix of #33 introduced a behavior change that may have not been intended. Although it looks logical as the recursion now actually matches the original type (see List<Ref> recurses instead of Ref recurses in comment in the test).