mtedone / podam

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

DataProviderStrategy.getMaxDepth(type) equals an actual depth minus 2 #241

Closed me-unsolicited closed 7 years ago

me-unsolicited commented 7 years ago

The max depth is not respected for indirectly recursive data structures.

For the test case below, the class Red has a Blue, which in turn has a Red. The custom provider sets the max depth to 1 for all types. This is the expected output.

MyTest.Red(blue=MyTest.Blue(red=null))

This is the actual output.

MyTest.Red(blue=MyTest.Blue(red=MyTest.Red(blue=MyTest.Blue(red=MyTest.Red(blue=MyTest.Blue(red=null))))))

Reproducible code sample (using lombok):

public class MyTest {

    @Data
    private class Red {
        private Blue blue;
    }

    @Data
    private class Blue {
        private Red red;
    }

    @Test
    public void test() {
        System.out.println(new MyPodamFactory().manufacturePojo(Red.class));
    }
}
public class MyPodamFactory extends PodamFactoryImpl {

    public MyPodamFactory() {
        super(new MyDataProviderStrategy());
    }
}
public class MyDataProviderStrategy extends AbstractRandomDataProviderStrategy {

    @Override
    public int getMaxDepth(Class<?> type) {
        return 1;
    }
}
daivanov commented 7 years ago

It actually respects the max depth value, the only problem is that an actual depth is declared depth + 2. Also note that currently Podam uses the same max depth 1, so your override of max depth in MyDataProviderStrategy even in theory could not lead to any changes. Thus, you can use max depth -1 to achieve your desired result.

Also I'm not sure why you are extending the factory. As a person using lombok you must go mental seeing unnecessary code ;)

Not sure how this confusion with depth happened. I will change it in next release.

me-unsolicited commented 7 years ago

Thanks, setting it to -1 resolved the issue. I suppose it didn't really have anything to do with the recursive data structure after all.

daivanov commented 7 years ago

Why? It does. Max depth + 2 is amount of dependency cycles, which will be filled until end it with null reference. When you specify 1, it will fill 3 cycles, when -1 it will fill 1.

me-unsolicited commented 7 years ago

I meant to say it doesn't seem to matter that the recursion is indirect. I got that idea by reading PodamFactoryImpl.java without really understanding it.

My mistake for closing the issue. I'm still figuring out github etiquette.

daivanov commented 7 years ago

Yes, depth is about cycles, not actual objects. For example, 2 cycles: A-A-null A-B-A-B- null A-B-C-A-B-C-null and so on... This is actually quite complex topic as some people want for example this: A0-B0-A0-... i.e. infinite loop instead of null ending sequence.

I just need this issue open as I didn't fix max depth value to reflect actual number of cycles.