Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RandomizerProxy.invoke will throw an exception if the Supplier passed to RandomizerProxy.asRandomizer is defined as package private or stricter #234

Closed
LucasAndersson opened this issue Feb 11, 2017 · 2 comments
Labels
Milestone

Comments

@LucasAndersson
Copy link
Contributor

LucasAndersson commented Feb 11, 2017

Hi

It was a bit hard to write a good title, I'll try my best to explain the issue.

I noticed that RandomizerProxy.invoke threw a IllegalAccessException when I passed a Supplier as a lambda to EnhancedRandomBuilder.randomize. I'm not completely sure but I think the java compiler desugars lambdas to anonymous inner classes which in turn are transformed into package private inner classes. If you change RandomizerProxyTest.theRandomizerProxyShouldBehaveLikeTheSupplier to use a lambda the test will pass. I think this is because RandomizerProxyTest and RandomizerProxy are both in the same package, if you move RandomizerProxyTest to another package the exception I was talking about earlier is thrown and the test fails.

You could just pass Randomizer instead of Supplier and everything would be fine but I like the semantics of the latter when I want to return something that isn't actually random and if you already support Supplier then you might as well support them as lambdas as well.

I hope you understood my explanation, it's a bit hard and confusing to explain. I have a solution for this problem and I'm ready to make a PR if you want. I think it will be easier to understand if you read my PR.

Thanks for this great library by the way, I use it all the time.

@PascalSchumacher
Copy link
Collaborator

Hi Lucas,

thanks for reporting.

If I understand you correctly the issue is that this:

@Test
public void test() {
    EnhancedRandom random = EnhancedRandomBuilder.aNewEnhancedRandomBuilder()
        .randomize(String.class, (Supplier<String>) ()-> "test").build();

    assertThat(random.nextObject(String.class)).isEqualTo("test");
}

fails with:

io.github.benas.randombeans.api.ObjectGenerationException: Unable to generate a random instance of type class java.lang.String
	at io.github.benas.randombeans.EnhancedRandomImpl.doPopulateBean(EnhancedRandomImpl.java:128)
	at io.github.benas.randombeans.EnhancedRandomImpl.nextObject(EnhancedRandomImpl.java:77)
	at com.test.RandomizerProxyTest.test(RandomizerProxyTest.java:41)
Caused by: java.lang.reflect.UndeclaredThrowableException
	at com.sun.proxy.$Proxy4.getRandomValue(Unknown Source)
	at io.github.benas.randombeans.EnhancedRandomImpl.doPopulateBean(EnhancedRandomImpl.java:99)
	... 25 more
Caused by: java.lang.IllegalAccessException: Class io.github.benas.randombeans.RandomizerProxy can not access a member of class com.test.RandomizerProxyTest$$Lambda$1/1100439041 with modifiers "public"
	at sun.reflect.Reflection.ensureMemberAccess(Reflection.java:102)

While this:

@Test
public void test() {
    EnhancedRandom randomizer = EnhancedRandomBuilder.aNewEnhancedRandomBuilder()
        .randomize(String.class, (Randomizer<String>) ()-> "test").build();

    assertThat(randomizer.nextObject(String.class)).isEqualTo("test");
}

works fine.

I think both ways should work.

A pull request with a fix is very welcome.

@LucasAndersson
Copy link
Contributor Author

LucasAndersson commented Feb 13, 2017

That is correct, with the exception that your first snippet actually works as long as it's located in the same package as RandomizerProxy. I'll create a PR once I'm home from work.

@fmbenhassine fmbenhassine added this to the 3.5.1 milestone Feb 14, 2017
fmbenhassine added a commit that referenced this issue Feb 14, 2017
issue #234 - RandomizerProxy should now be able to accept Supplier as lambda.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants