mtedone / podam

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

Fix wildcard enum npe #233

Closed Otanikotani closed 7 years ago

Otanikotani commented 7 years ago

When I switched from version 6.0.2.RELEASE to 7.0.4.RELEASE I noticed that some previously manufactured types started to throw exceptions. The reason was:

private HashMap<Enum<?>, Object> params = new HashMap<Enum<?>, Object>();

This PR changes the behavior of EnumTypeManufacturerImpl.java to return null when getEnumConstants() returns null. This might not be ideal because now the map is being populated with an entry with null key. I think that it would be better if it was just an empty map.

Also, I do not know the rules for adding unit tests to the project, I simply did it iteratively to what is in current version and put my unit tests to /pdm46.

daivanov commented 7 years ago

Thank you for the patch. Could you please move the test somewhere to PodamFactoryBasicTypesTest close to podamShouldFillPojoWithEnums()? Pdm46 is a bad name as it provides no context.

Otanikotani commented 7 years ago

Done.

I also noticed one thing when I migrated from 6.0.2 to 7.0.4. If I try to manufacture pojo like this one:

public class WithRawCollectionPojo implements Serializable {
    private java.util.ArrayList rawList;

    public ArrayList getRawList() {
        return rawList;
    }

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

It will throw an error:

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

which was not the case in previous versions. I work with a legacy platform and unfortunately cannot change fields to not to be raw typed. Any tips on what I can do about it? I would prefer the behaviour when such lists are simply not populated, I think.

daivanov commented 7 years ago

Please, make list issue a separate issue or PR. It's very hard to manage tickets where people bring lots of thing in.

daivanov commented 7 years ago

In the test this code

Enum<?> wildcardEnumField = pojo.getWildcardEnumField();
podamValidationSteps.thePojoShouldBeNull(wildcardEnumField);

should be something like this:

Enum<?> wildcardEnumField = pojo.getWildcardEnumField();
podamValidationSteps.thePojoMustBeOfTheType(wildcardEnumField, Enum.class);

And I can see this doesn't pass. Do you want to fix this?

And the test description should be "Podam should should fill in wildcard Enum fields".

Otanikotani commented 7 years ago

The idea was to make wildcard enum fields ignored, so:

Enum<?> wildcardEnumField = pojo.getWildcardEnumField();
podamValidationSteps.thePojoShouldBeNull(wildcardEnumField);

is what I see as a correct behaviour since it simply cannot be populated with any value except null. So, I believe the description is also correct because Podam simply cannot fill wildcard Enum fields.

Otanikotani commented 7 years ago

About rawList - I will try to figure out if there is a way I can fix it myself. If not - I will create an issue, otherwise I'd prefer to make another PR.

Otanikotani commented 7 years ago

Opened issue about rawList: https://github.com/devopsfolks/podam/issues/234

daivanov commented 7 years ago

it simply cannot be populated with any value except null.

Impossible is nothing ;)

Otanikotani commented 7 years ago

Well, I am by no means a master of manufacturing random pojos, but what you can really put into Enum<?> field? Some synthetic enum or something like that?

daivanov commented 7 years ago

Merged. Thank you very much for reporting this issue and the patch.