Skip to content

ControllerLinkBuilder.linkTo fails to build a link to a method that uses entity as a target of @PathVariable in its signature #352

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

Closed
vtsukur opened this issue May 31, 2015 · 12 comments
Assignees
Milestone

Comments

@vtsukur
Copy link

vtsukur commented May 31, 2015

Consider a project using Spring Data JPA / Spring Data REST to manage an entity MyEntity via repository annotated with @RepositoryRestResource:

@RepositoryRestResource
public interface MyEntityRepository extends PagingAndSortingRepository<MyEntity, Long> {}

Consider the following (custom) Spring MVC controller in such an application:

@RestController
@RequestMapping("/path/{id}/subpath")
public class MyResourceController {

    @RequestMapping(method = RequestMethod.GET)
    public ... getSomething(@PathVariable("id") MyEntity myEntity) ...

}

and the following ResourceProcessor:

@Component
public class MyResourceProcessor implements ResourceProcessor<Resource<MyEntity>> {

    @Override
    public Resource<MyEntity> process(Resource<MyEntity> resource) {
        resource.add(linkTo(methodOn(MyResourceController.class).getSomething(resource.getContent())).withRel("payment"));
        return resource;
    }

}

ResourceProcessor will fail to generate the link avoiding the knowledge and capabilities of DomainClassConverter and @PathVariable

org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type @org.springframework.web.bind.annotation.PathVariable org...domain.MyEntity to type java.lang.String
        at org.springframework.core.convert.support.GenericConversionService.handleConverterNotFound(GenericConversionService.java:313)
        at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:195)
        at org.springframework.hateoas.mvc.AnnotatedParametersParameterAccessor$BoundMethodParameter.asString(AnnotatedParametersParameterAccessor.java:172)
        at org.springframework.hateoas.mvc.ControllerLinkBuilderFactory.linkTo(ControllerLinkBuilderFactory.java:130)
        at org.springframework.hateoas.mvc.ControllerLinkBuilder.linkTo(ControllerLinkBuilder.java:136)
        at org...MyResourceProcessor.process(BookingProcessor.java:25)
        at org...MyResourceProcessor.process(BookingProcessor.java:16)
@gregturn
Copy link
Contributor

gregturn commented May 31, 2015

For a Spring data rest project, you should not use RestController. See spring-projects/spring-data-rest#177 for documentation updates on the proper way to build overriding controllers.

@vtsukur
Copy link
Author

vtsukur commented May 31, 2015

Thanks a lot for the quick turnaround and the hint on @RepositoryRestController to override controllers. This is going to be useful in many cases and I already started using it ;-).

Still, it does not solve the initial concern of having an ability to generate links to methods where @PathVariables help to resolve entities - builder will fail for the very same reason even if @RepositoryRestController is used. Using identifier instead of the entity and repository lookup would solve it of course (that is what I do right now), but it would be better if @PathVariable semantics works with ControllerLinkBuilder.

@gregturn
Copy link
Contributor

I don't understand how @PathVariable("id") MyEntity myEntity is supposed to map onto /path/{id}/subpath. Looks like {id} should map to Long id, and plugging in resource.getContent().getId() to invoke it would line up cleaner.

@gregturn
Copy link
Contributor

To clarify on the @RepositoryRestController stuff, the entire @RequestMapping needs to be on the method and none of it at the class level.

@vtsukur
Copy link
Author

vtsukur commented May 31, 2015

Spring Data JPA reference documentation says that DomainClassConverter allows to use domain types in Spring MVC controller method signatures, exactly like this: @PathVariable("id") MyEntity myEntity. Same mapping works perfectly well for @RepositoryRestControllers. It is only the link generation which fails.

@PathVariable("id") MyEntity myEntity is supposed to map onto /path/{id}/subpath by serializing identifier of the entity: /path/5/subpath if entity id is 5. The need to map {id} to Long id and then play around with resource.getContext().getId() is exactly what I am trying to avoid because it can potentially be done by the framework itself. Does it make sense?

@aglassman
Copy link

Just ran into this issue as well. I have a controller that has MyDomainId as a parameter. I have registered a Converter<String,MyDomainId>, but when passing the instance of MyDomainId into the linkTo method, I receive the same error @vtsukur posted above. Registering a Converter<MyDomainId,String> converter did not solve the issue.

FYI I'm not using SpringData, just Spring Hateoas.

@rgacki
Copy link

rgacki commented Aug 27, 2017

I have a similar issue. It seems to be the same for other annotations like @RequestParam. I am using custom types along with org.springframework.core.convert.converter.Converter implementations. org.springframework.hateoas.mvc.AnnotatedParametersParameterAccessor.BoundMethodParameter is using a self-instantiated ConversionService. This service does not see any of the custom converters registered elsewhere.

This is a bad show stopper for me!

gregturn added a commit that referenced this issue Aug 28, 2017
Existing ConversionService is fixed as a static inside BoundMethodParameter. This introduces ability to inject an alternative ConversionService.

Related issues: #352
@gregturn
Copy link
Contributor

The issue is that the existing conversion service is baked into Spring HATEOAS. There is currently no means to pick up and inject your own.

@gregturn gregturn self-assigned this Aug 28, 2017
gregturn added a commit that referenced this issue Aug 28, 2017
Existing ConversionService is fixed as a static inside BoundMethodParameter. This introduces ability to inject an alternative ConversionService.

Related issues: #352, #149
@leonslack
Copy link

Hello @vtsukur
You should use linkTo Controller passing the pathVariable property like
linkTo(MyResourceController .class, id) instead of linkToMethod

@trungie
Copy link

trungie commented Jan 11, 2018

I've been able to get around this error using a hack - creating a String based constructor on the object with a toString()....

This will get picked up by a default converter FallBackObjectToStringConverter

I use a Hashid based argument in a controller method, encapsulated in an object (similar concept to @aglassman's MyDomainId) and it's annoying i have to use this hack till a real solution is out...

@Kidlike
Copy link

Kidlike commented Jun 14, 2019

According to this thread, these cases are not working:

  • @PathVariable
  • @RequestParam

To add to the list, I have another case that is not working.
If a controller method has aggregated query params like so:

@GetMapping("/find")
public ... findSomething(FindParams findParams) ...
public class FindParams {
    private String name;
    private String type;
    private boolean includeSomething;
    // etc...
}

Hateoas in this case would completely ignore any FindParams objects you pass to it.
It will always generate just a /find URL.

This might even be more dangerous than the previously mentioned issues, because there's no exception.


Is any of this fixable?!

@odrotbohm odrotbohm assigned odrotbohm and unassigned gregturn Jan 25, 2021
@odrotbohm odrotbohm added this to the 1.3 M2 milestone Jan 25, 2021
@odrotbohm
Copy link
Member

Duplicates #118.

odrotbohm added a commit that referenced this issue Jan 25, 2021
We now look up the ConversionService available in the ApplicationContext from Web(Mvc|Flux)LinkBuilder. Some API tweaks to WebHandler to allow the lookup from the current request. The general fallback is now the invocation of …toString() on the parameter value.

Fixes #118, #352, #144, #149.
odrotbohm added a commit that referenced this issue Jan 25, 2021
We now look up the ConversionService available in the ApplicationContext from Web(Mvc|Flux)LinkBuilder. Some API tweaks to WebHandler to allow the lookup from the current request. The general fallback is now the invocation of …toString() on the parameter value.

Fixes #118, #352, #144, #149.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants