mtedone / podam

PODAM - POjo DAta Mocker
https://mtedone.github.io/podam
MIT License
323 stars 750 forks source link

Podam fails to find a right manufacturer, if several manufacturers are matching POJO's type hierarchy #234

Closed Otanikotani closed 7 years ago

Otanikotani commented 7 years ago

Pojo:

    public class Foo {
        ArrayList rawArrayList;

        public ArrayList getRawArrayList() {
            return rawArrayList;
        }

        public void setRawArrayList(ArrayList rawArrayList) {
            this.rawArrayList = rawArrayList;
        }
    }

Test:

public class FooTest {
    @Test
    public void test_() throws Exception {
        PodamFactoryImpl podamFactory = new PodamFactoryImpl();
        podamFactory.manufacturePojo(Foo.class);
    }
}

The test fails with exception:

java.lang.IllegalArgumentException: java.util.ArrayList is missing generic type arguments, expected [E], provided []

The error message makes a lot of sense - a raw type is used and the factory cannot figure out what to populate the list with. However, this:

    public class Foo {
        List rawList;

        public List getRawList() {
            return rawList;
        }

        public void setRawList(List rawList) {
            this.rawList = rawList;
        }
    }

works! It gives the warning:

WARN  uk.co.jemos.podam.typeManufacturers.TypeManufacturerUtil - Unrecognized type E. Will use Object instead

but it doesn't throw an exception. I see it as inconsistency, since one would expect ArrayList to work the same way List works.

After lurking the code, I think I found the problem. In AbstractRandomDataProviderStrategy.getTypeValue():

Deque<Class<?>> types = new ArrayDeque<Class<?>>();
        types.add(pojoType);
        TypeManufacturer<?> manufacturer = null;
        while (null == manufacturer && !types.isEmpty()) {

            Class<?> type = types.remove();
            manufacturer = typeManufacturers.get(type);
            if (null == manufacturer) {
                for (Class<?> iface : type.getInterfaces()) {
                    types.add(iface);
                }
                type = type.getSuperclass();
                if (null != type) {
                    types.add(type);
                }
            }
        }

It tries to find the best manufacturer for the given type. When it looks for manufacturer for type List it finds the CollectionTypeManufacturerImpl. However, for ArrayList it finds ArrayTypeManufacturerImpl! As far as I understood it happens because when it looks for the best manufacturer it does it breadth-first over interfaces implemented by the type. So for List it goes straight to looking for manufacturer for Collection type and finds it. For ArrayList:

public class ArrayList<E> extends AbstractList<E>
        implements List<E>, RandomAccess, Cloneable, java.io.Serializable

it looks for manufacturer for Cloneable before looking for manufacturer for Collection since the latter is deeper in interface hierarchy.

I think breadth-first approach is better in general, but in this particular case it failed. I don't see an easy solutions which can be done in the library without hardcoded exceptions. Please share if you see how to solve it. For my need I simply registered a custom manufacturer like this:

        podamFactory.getStrategy().addOrReplaceTypeManufacturer(ArrayList.class, (strategy, attributeMetadata, genericTypesArgumentsMap) -> new ArrayList<>());

and it worked for me. But I wanted to share the problem anyways, if you think it is not a bug, please close it.

daivanov commented 7 years ago

Snapshot 7.0.5-2.

daivanov commented 7 years ago

I will resolve this issue, if you don't mind.

Otanikotani commented 7 years ago

Okay, thank you

daivanov commented 7 years ago

Thank you for reporting.

daivanov commented 7 years ago

Released in Podam 7.0.5

InternetPseudonym commented 7 years ago

Um .... nope, its not released, im getting a very similar exception:

java.lang.IllegalArgumentException: java.util.List is missing generic type arguments, expected [E], provided []
    at uk.co.jemos.podam.typeManufacturers.TypeManufacturerUtil.fillTypeArgMap(TypeManufacturerUtil.java:169)
    at uk.co.jemos.podam.api.PodamFactoryImpl.manufacturePojoInternal(PodamFactoryImpl.java:499)
    at uk.co.jemos.podam.api.PodamFactoryImpl.doManufacturePojo(PodamFactoryImpl.java:436)
    at uk.co.jemos.podam.api.PodamFactoryImpl.manufacturePojo(PodamFactoryImpl.java:151)
daivanov commented 7 years ago

Hi, which version of podam are you using and could you please show a POJO resulting in this exception? From my side I could say that 7.0.5 is definitely released.

InternetPseudonym commented 7 years ago

sure but im really busy atm and i would need to anonymize the classes first ... you may expect them in the next 7 days or so

daivanov commented 7 years ago

It's enough to have a very schematic POJO, just like Andrey provided, which reproduces the problem.

public class Foo {
    ArrayList rawArrayList;

    public ArrayList getRawArrayList() {
        return rawArrayList;
    }

    public void setRawArrayList(ArrayList rawArrayList) {
        this.rawArrayList = rawArrayList;
    }
}

The exception is related to a single field and how it's defined in terms of constructors/setting/visibility. You can see the POJO, which caused the problem from the log on debug level.

Could you also confirm you are using Podam 7.0.5?

InternetPseudonym commented 7 years ago

Im currently on another version because we still rely on older API but i successfully reproduced the problem on 7.0.5, yes.

daivanov commented 7 years ago

So did you have any chance to find the POJO, which is causing problems?

InternetPseudonym commented 7 years ago

not yet, sorry - we are being swamped with work ... ill get on it sometime if the problem still exists with current versions