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

Unable to use Kotlin enums with @Validated beans [SPR-16931] #21470

Closed
spring-issuemaster opened this issue Jun 11, 2018 · 7 comments

Comments

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

commented Jun 11, 2018

Daniel Jones opened SPR-16931 and commented

I have a configuration properties kotlin class with a kotlin enum property. If the class is annotated with @Validated an exception occurs in hibernate-validator's ParameterMetadata class due to an IndexOutOfBoundsException.

This seems to happen due to the underlying ParameterNameDiscoverer, KotlinReflectionParameterNameDiscoverer.

I think this is due to the validator class expecting a 2-parameter constructor (name, ordinal) but KotlinReflectionParameterNameDiscoverer returns an empty array.

Example configuration properties class:

@ConfigurationProperties("my.prefix")
@Validated
data class MyProperties(        
        var enumProp: MyEnum = MyEnum.ONE

) {
    enum class MyEnum {
        ONE, TWO
    }
} 

Affects: 5.0.6

Referenced from: commits 2c5a1af, 73db208

Backported to: 5.0.11

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 21, 2018

Sébastien Deleuze commented

Indeed, I have been able to reproduce this issue. Not sure what we do wrong here, it could be a Kotlin reflection bug or a conceptual difference between Java and Kotlin. Any thoughts Daniel Jones?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 28, 2018

Sébastien Deleuze commented

I have created KT-25165.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 28, 2018

Daniel Jones commented

Sorry, missed the notification from 6 days ago. I'm not really sure other than I remembering getting different results in the debugger if I used Kotlin's reflection API over Java's.

A quick go on the Kotlin REPL in IntelliJ gives me the following differences:

Using the Kotlin API, this returns an empty array

// toTypeArray().size == 1
MyEnum::class.constructors.toTypedArray()[0].parameters
>> []

Using the Java API, I get the expected 2-arg constructor

// declaredConstructors.size == 1
MyEnum::class.java.declaredConstructors[0]
>> protected my.package.MyEnum(java.lang.String,int)

So I think you're on the right track that the issue is with Kotlin.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 9, 2018

ruslanys commented

Sébastien Deleuze

I'm sorry for disturbing you, but I have the same issue and tried to solve it.

So, can we tweak KotlinReflectionParameterNameDiscoverer to solve this issue?

In my perspective, looks that all in our hands and we don't need to wait for something from Kotlin.

Looks, like if we add checking for enums, it will solve the issue.
I changed the method with the following and it works for me.

@Override
@Nullable
public String[] getParameterNames(Constructor<?> ctor) {
	if (ctor.getDeclaringClass().isEnum() || !KotlinDetector.isKotlinType(ctor.getDeclaringClass())) {
		return null;
	}

	try {
		KFunction<?> function = ReflectJvmMapping.getKotlinFunction(ctor);
		return (function != null ? getParameterNames(function.getParameters()) : null);
	}
	catch (UnsupportedOperationException ex) {
		return null;
	}
}

According to PrioritizedParameterNameDiscoverer.getParameterNames(Constructor<?> ctor) method, ParameterNameDiscoverer s will be applied until one of them won't return null.

So, when KotlinReflectionParameterNameDiscoverer returns null, then the following StandardReflectionParameterNameDiscoverer will be applied and return "$enum$name", "$enum$oridnal" parameters for default enum and this won't cause an error.

And looks like Alexander Udalov from JetBrains had clear clarification: for Kotlin language, Enum doesn't have undeclared parameters.

So, because of this, don't you think that for Kotlin Enums we should delegate parameters discovering to Java?
Can we use the suggested approach?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 9, 2018

ruslanys commented

I've added a PR-1985, please take a look.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2018

Sébastien Deleuze commented

Thanks I will have a look asap.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2018

Sébastien Deleuze commented

Fix in master and 5.0.x branches, I think indeed this is the correct approach. Thanks for your feedback and help on that issue.

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.