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

Repository method override not picked up #2888

Open
sbont opened this issue Jul 22, 2023 · 5 comments
Open

Repository method override not picked up #2888

sbont opened this issue Jul 22, 2023 · 5 comments
Assignees
Labels
in: kotlin Kotlin support status: waiting-for-triage An issue we've not yet triaged

Comments

@sbont
Copy link

sbont commented Jul 22, 2023

Using Spring Boot Data 3.0.6, with Kotlin.
I have a UserRepository extending a CrudRepository, with a method override

@RestResource
override fun findById(id: Long): Optional<User>

My default method exposure is disabled.
When I call my REST endpoint on /users/123 I get a 405 Method Not Allowed. If I enable method exposure, the default implementation from CrudRepository is being picked up instead of my custom declared repository method.

If I declare a custom findAll(), this gets picked up without issues.
I did some digging and it seems that in DefaultCrudMethods.selectMostSuitableFindOneMethod(...) it fails to find the custom interface method, it however does find the CrudRepository one. It calls ReflectionUtils.findMethod(UserRepository.class, "findById", Long.class), and there the comparison of parameter types somehow fails.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 22, 2023
@christophstrobl christophstrobl transferred this issue from spring-projects/spring-data-jpa Jul 24, 2023
@christophstrobl christophstrobl transferred this issue from spring-projects/spring-data-rest Jul 24, 2023
@gregturn gregturn assigned christophstrobl and unassigned gregturn Jul 31, 2023
@christophstrobl
Copy link
Member

The signatures do not match because the id parameter of CrudRepository is considered non nullable, thus the Kotlin signature of the override uses long instead of Long.

@christophstrobl christophstrobl added the in: kotlin Kotlin support label Aug 1, 2023
@christophstrobl
Copy link
Member

We could change the lookup to something like the following which worked for the given example.

return Optional.ofNullable(ReflectionUtils.findMethod(type, name, parameterTypes)).or(() -> {
    return stream(type.getDeclaredMethods()).filter(it -> it.getName().equals(name) && it.getParameterCount() == parameterTypes
            .filter(it -> {
                for(int i = 0; i<it.getParameterCount(); i++) {
                    Class<?> currentParameterType = it.getParameterTypes()[i];
                    if(currentParameterType != parameterTypes[i] && ClassUtils.isPrimitiveWrapper(currentParameterType)) {
                        if(!ClassUtils.isAssignable(parameterTypes[i], currentParameterType)) {
                            return false;
                        }
                    }
                }
                return true;
            })
            .findFirst();
});

@mp911de mp911de self-assigned this Aug 7, 2023
@KimJejun
Copy link

can u try like this?

@Repository
@JvmDefaultWithCompatibility
inteface UserRepository: CrudRepository<User, Long> {
    override fun findAll(): MutableIterable<User> {
        throw UnsupportedOperationException()
    }
}

@christophstrobl Is working progress?

@sttomm
Copy link

sttomm commented Jul 30, 2024

I am struggling with the same issue. I first disabled Kotlin warnings as errors and defined the parameter as nullable findById(@Param("id") id: Long?). With that change the overriden findById works fine and I can access the endpoint. But I now finally want to enable warnings again and can't find a working solution.

What I am doing regarding customization is adding a PreAuthorize annotation to it.

@RestResource
@PreAuthorize("hasAuthority('MY_AUTHORIY')")
override fun findById(@Param("id") id: Long): Optional<PlatformInstanceNotification>

@xanscale
Copy link

xanscale commented Nov 25, 2024

after some hours of experiment the best solution i found is to use UUID :)

@Id @GeneratedValue(strategy = GenerationType.UUID) val id: UUID,

and it works perfectly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: kotlin Kotlin support status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

8 participants