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

BridgeMethodResolver#isBridgeMethodFor return incorrect result for kotlin code in certain circumstance #26585

Closed
noob9527 opened this issue Feb 22, 2021 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@noob9527
Copy link

noob9527 commented Feb 22, 2021

Affects: v5.3.4

In kotlin, non-nullable Int is compiled to java primitive type int, this cause the condition candidateParameter.equals(genericParameter.toClass() always evaluates to false. so org.springframework.core.BridgeMethodResolver#findBridgedMethod fails to find the correct bridge method.

screenshot1

import org.springframework.stereotype.Repository
import org.springframework.transaction.annotation.Transactional

interface GenericInterface<ID> {
    fun delete(id: ID)
}

abstract class AbstractGenericClass<ID> : GenericInterface<ID> {
    override fun delete(id: ID) {
    }
}

@Repository
class GenericRepository : AbstractGenericClass<Int>() {
    @Transactional
    override fun delete(
            id: Int
    ) {
        error("gotcha")
    }
}

the above "GenericRepository" is compiled to

@Repository
@Metadata(
   mv = {1, 4, 0},
   bv = {1, 0, 3},
   k = 1,
   d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0018\u0002\n\u0002\u0010\b\n\u0002\b\u0002\n\u0002\u0010\u0002\n\u0002\b\u0002\b\u0017\u0018\u00002\b\u0012\u0004\u0012\u00020\u00020\u0001B\u0005¢\u0006\u0002\u0010\u0003J\u0010\u0010\u0004\u001a\u00020\u00052\u0006\u0010\u0006\u001a\u00020\u0002H\u0017¨\u0006\u0007"},
   d2 = {"Lcom/cpvsn/crud/demo/feat/generic/GenericRepository;", "Lcom/cpvsn/crud/demo/feat/generic/AbstractGenericClass;", "", "()V", "delete", "", "id", "crud-kit.crud-kit-demo.main"}
)
public class GenericRepository extends AbstractGenericClass {
   @Transactional
   public void delete(int id) {
      String var2 = "gotcha";
      boolean var3 = false;
      throw (Throwable)(new IllegalStateException(var2.toString()));
   }

   // $FF: synthetic method
   // $FF: bridge method
   public void delete(Object var1) {
      this.delete(((Number)var1).intValue());
   }
}

Due to this issue, the @Transactional on delete(id: Int) method have no effects at all.
I'm not sure if this project is expected to be compatible with Kotlin code. If it is, then this is a bug, I think I can try to submit a PR to fix it.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 22, 2021
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 22, 2021
@sdeleuze sdeleuze added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 22, 2021
@sdeleuze sdeleuze added this to the 5.3.5 milestone Feb 22, 2021
@sdeleuze sdeleuze self-assigned this Feb 22, 2021
@sdeleuze sdeleuze modified the milestones: 5.3.5, 5.3.6 Mar 16, 2021
@sdeleuze
Copy link
Contributor

@jhoeller I have little experience with bridge methods, but based on my initial findings with this test, my current thinking is that we could add Kotlin specific code path in BridgeMethodResolver#isResolvedTypeMatch.

Instead of just doing Type[] genericParameters = genericMethod.getGenericParameterTypes(), we should maybe use KClass<T>.javaObjectType in order to get a type that will be compatible with the rest f the logic. In the use case we have here, java.lang.Integer would be returned for both Kotlin Int and Int? types. But I am not sure KClass<T>.javaObjectType is exposed in Java, to be checked with Kotlin team.

I think you mentioned another potential way to fix that, but I can't remember the details. Any thoughts?

@noob9527 Since you mentioned a PR, any thoughts on what could look like the fix?

@noob9527
Copy link
Author

@sdeleuze
I think ResolvableType.forMethodParameter(genericMethod, i, declaringClass); will always return a wrapper type.
so we can just wrap candidateParameter to its wrapper type before comparing.

Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this issue Aug 20, 2021
This commit adds support for Kotlin non-nullable type which resolves
to primitive Java types in BridgeMethodResolver#findBridgedMethod.

Closes spring-projectsgh-26585
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
This commit adds support for Kotlin non-nullable type which resolves
to primitive Java types in BridgeMethodResolver#findBridgedMethod.

Closes spring-projectsgh-26585
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants