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

Kotlin inner class nested configuration causes IndexOutOfBoundsException [SPR-17222] #21755

Closed
spring-issuemaster opened this issue Aug 28, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Aug 28, 2018

Hugh Hamill opened SPR-17222 and commented

With Reference to the following project which reproduces the issue

https://github.com/hughwphamill/spring-nested-test-configuration-issue

Problem

Application Context fails to load with java.lang.IndexOutOfBoundsException where there is a nested inner @Configuration class in a Kotlin project

Cause

MethodParameter.java

if (function != null) { 
    List<KParameter> parameters = function.getParameters(); 
    KParameter parameter = parameters
        .stream()
        .filter(p -> KParameter.Kind.VALUE.equals(p.getKind()))
        .collect(Collectors.toList()) 
        .get(index); 
        return (parameter.getType().isMarkedNullable() || parameter.isOptional()); 
}

When checking for required constructor parameters in DependencyDescriptor the statement above filters out Parameter 0 which is a reference to the outer class and has kind value of INSTANCE.

It then attempts to access the parameter based on the initial input index, but the stream has been filtered down to empty.

 

java.lang.IllegalStateException: Failed to load ApplicationContext

at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:125)
 at org.springframework.test.context.support.DefaultTestContext.getApplicationContext(DefaultTestContext.java:108)
 at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.injectDependencies(DependencyInjectionTestExecutionListener.java:117)
 at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.prepareTestInstance(DependencyInjectionTestExecutionListener.java:83)
 at org.springframework.boot.test.autoconfigure.SpringBootDependencyInjectionTestExecutionListener.prepareTestInstance(SpringBootDependencyInjectionTestExecutionListener.java:44)
 at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:246)
 at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.createTest(SpringJUnit4ClassRunner.java:227)
 at org.springframework.test.context.junit4.SpringJUnit4ClassRunner$1.runReflectiveCall(SpringJUnit4ClassRunner.java:289)
 at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
 at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.methodBlock(SpringJUnit4ClassRunner.java:291)
 at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:246)
 at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:97)
 at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
 at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
 at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
 at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
 at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
 at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
 at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
 at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
 at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:190)
 at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
 at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
 at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
 at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
 at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
 Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'springNestedTestConfigurationIssueApplicationLoadFailure.NestedTestConfiguration': Unexpected exception during bean creation; nested exception is java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
 at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:508)
 at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:317)
 at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:222)
 at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:315)
 at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
 at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:759)
 at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:869)
 at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:550)
 at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:762)
 at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:398)
 at org.springframework.boot.SpringApplication.run(SpringApplication.java:330)
 at org.springframework.boot.test.context.SpringBootContextLoader.loadContext(SpringBootContextLoader.java:139)
 at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:99)
 at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:117)
 ... 25 more
 Caused by: java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
 at java.util.ArrayList.rangeCheck(ArrayList.java:657)
 at java.util.ArrayList.get(ArrayList.java:433)
 at org.springframework.core.MethodParameter$KotlinDelegate.isOptional(MethodParameter.java:774)
 at org.springframework.core.MethodParameter.isOptional(MethodParameter.java:342)
 at org.springframework.beans.factory.config.DependencyDescriptor.isRequired(DependencyDescriptor.java:174)
 at org.springframework.beans.factory.support.SimpleAutowireCandidateResolver.isRequired(SimpleAutowireCandidateResolver.java:40)
 at org.springframework.beans.factory.annotation.QualifierAnnotationAutowireCandidateResolver.isRequired(QualifierAnnotationAutowireCandidateResolver.java:321)
 at org.springframework.beans.factory.support.DefaultListableBeanFactory.isRequired(DefaultListableBeanFactory.java:1231)
 at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1100)
 at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1062)
 at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:818)
 at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:724)
 at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:197)
 at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1267)
 at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1124)
 at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:535)
 at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:495)
 ... 38 more

Affects: 5.0.8

Reference URL: https://github.com/hughwphamill/spring-nested-test-configuration-issue

Referenced from: commits 8d45e3e, 89fca1b

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2018

Sébastien Deleuze commented

I have a fix for MethodParameter, but it leads to a NoSuchBeanDefinitionException exception because inner classes require a reference to an instance of the outer class. We could not providing such constructor parameter, but the reference to the outer class would then be null and could lead to NPE if used by the user, so I don't think it is a good idea to do that.

Conceptually I am not sure we should support inner configuration classes (just nested ones), so I am tempted to do only the MethodParameter fix and document that configuration classes can be nested but not inner.

Any thoughts Hugh Hamill?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 7, 2018

Hugh Hamill commented

Hi Sébastien Deleuze I'm curious as to the intent of applying the filter before executing the 'get' based on the index which could potentially be changed by the filter?

If the filter doesn't change the index, then there's no point in applying it, and if it does change it you're always going to be exposed to either no such element or potentially 'get'ting the wrong param?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 7, 2018

Sébastien Deleuze commented

The updated filter avoid removing the inner class first constructor of kind INSTANCE as shown in that commit, leading to valid index and throwing later a more meaningful NoSuchBeanDefinitionException. Does that make sense for you?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 7, 2018

Hugh Hamill commented

 OK so I get that the outer class is not available when attempting to constructor wire the inner class, as there's no bean of that type.  I think you're right that the correct answer is a more meaningful error that lets the user know that inner classes can't be supported unless the outer class is itself a bean that is autowireable.

That said, I'm still worried about that filter followed by get.

If the filter changes the index of the paramter, then it will break something, if it doesn't change the index then I don't see the purpose of applying it?

 

Imagine this case

 

[param1, param2, param3]

 

We're checking for param2. (index 1)

 

Scenario 1:

Filter removes param1, when we call get(1) we retrieve param3, when we wanted param2?

 

Scenario 2:

Filter removes param2, when we call get(1) we retrieve param3 because param2 is gone.

 

Scenario 3:

Filter removes param3, when we call get(1) we retrieve param2, as desired

 

Scenario 4:

Filter removes all params, when we call get(1) we throw index out of bounds.

 

Scenario 5:

Filter removes no params, when we call get(1) we retrieve param2, as desired.

 

There's conceptually no difference between scenario 3 and scenario 5,

Scenario 1 and scenario 2 lead to unexpected behaviour

Scenario 4 leads to a runtime exception

 

I can't see the purpose of the filter, it only introduces instability?

 

Help me see what I'm missing or not understanding!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 7, 2018

Sébastien Deleuze commented

The filter is needed to avoid getting INSTANCE and EXTENSION_RECEIVER parameter kind for member function in order to have consistency in parameter indexes between Java and Kotlin.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 7, 2018

Hugh Hamill commented

Ah ok, so we strip out these Kotlin params from the start of the function and what's left over is supposed to be the same signature as the equivalent java method?

 

Just did a bit of research, might this be a potentially safer alternative?

 

https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.reflect.full/value-parameters.html

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 7, 2018

Hugh Hamill commented

Actually, that's really just doing what you're doing, looking at the source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.