mtedone / podam

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

MaxDepth not working with multiple fields #263

Closed shollander closed 6 years ago

shollander commented 6 years ago

I have an object with the following structure:

public class MyObject {
    private String name;
    private MyObject related;
    private List<MyObject> children;
}

The default maxDepth setting of 3 is too much for me, I would like the objects to recurse only once (i.e. only related and children should be populated, but related.related and related.children, and children[*].related and children[*].children should be empty. So I created a custom DataProviderStrategy and overrode the getMaxDepth() method to return 2 instead of the default 3. As follows:

public class MyDataProviderStrategy extends AbstractRandomDataProviderStrategy {
   public MyDataProviderStrategy() {
      super.setMemoization(false);
   }

  @Override
  public int getMaxDepth(Class<?> type) {
    if(MyObject.class.isAssignableFrom(type)) {
      return 2;
    } else {
      return 1;
    }
}

What happened was that the related field was correct (only 1 level of recursion), but the children field was incorrect. It was always empty even at the top level.

It also seems to me that there should be an easier way to configure maxDepth than overriding the method. Something like setMaxDepth(Class<?>, int) would be helpful.

daivanov commented 6 years ago

In Podam 7.1.1 I cannot reproduce this problem.

shollander commented 6 years ago

I just tried it again using Podam 7.1.1, and the problem is definitely still there. I should clarify that the children field is not actually empty, it is actually populated by Podam with an ArrayList of all nulls. If I change the implementation of getMaxDepth() to read as follows:

 @Override
 public int getMaxDepth(Class<?> type) {
    if(MyObject.class.isAssignableFrom(type) || ArrayList.class.isAssignableFrom(type)) {
      return 2;
    } else {
      return 1;
    }
}

then the children field gets populated correctly. But this does not appear to be correct since that will catch any ArrayList, not just my List<MyObject> field.

daivanov commented 6 years ago

Hi,

Could you please share a log on debug level?

Thanks, Daniil

shollander commented 6 years ago

Hi,

I have created a sample project with a failing unit test here: https://github.com/shollander/podam-test

Just execute ./gradlew test -i and watch the unit test fail.

Let me know if you have any questions.

Thanks!

daivanov commented 6 years ago

As you can see test is passing successfully. I absolutely fail to see what is being reported in this issue.

But this does not appear to be correct since that will catch any ArrayList, not just my List field.

I cannot really say, why this would not be correct.

shollander commented 6 years ago

I don't see that the test is passing successfully. Did you clone the test in the repo mentioned above and execute the test through gradle (https://github.com/shollander/podam-test)? This is what I get (using the -i flag as mentioned above to show the assertion stackrace info):

image

As far as the second comment:

But this does not appear to be correct since that will catch any ArrayList, not just my List field.

I cannot really say, why this would not be correct.

What I meant was that while I can get it working by forcing a depth of 2 for all ArrayList objects, it is not correct in the sense that I don't want to apply the rule that generally. I really only want it to apply to the MyObject class (including List<MyObject>), but if I have a different list of some other type I don't necessarily want it to be included in the same rule.

daivanov commented 6 years ago

If you for whatever reason want to have different instantiation of Lists, then extend AbstractTypeManufacturer for the types in question and handle it the way you like.

Thanks, Daniil

shollander commented 6 years ago

Daniil, I fail to see how AbstractTypeManufacturer would help me in this case. As far as I can tell, that only controls how the type would be created. I don't think that it would help with controlling recursion or depth.

To be clear, what I am talking about is that the test case you added should still pass when the else clause at PodamFactorySteps.java:175 is removed.

Since I want to limit the level of recursion for the RecursivePojoWithList class, I should not be specifying any other classes. The List class is not a RecursivePojoWithList, it only contains RecursivePojoWithList objects, so I should only be specifying RecursivePojoWithList.

daivanov commented 6 years ago

If else clause is removed, then loop depth is 1, meaning any children will be left as nulls. And this is exactly what is happening.

TypeManufacturer receives AttributeMetadata, which contain information about parent POJO and reference to it. This allows to do handle recursions of the POJO hierarchy in a pretty arbitrary way.

daivanov commented 6 years ago

If you don't mind I will close this issue.

Thanks, Daniil