BoundMethodParameter references static, default instance of ConversionService

[nebhale]

Currently the BoundMethodParameter class references a static, default instance of ConversionService. This means that should a user add Converters to their ApplicationContext they will not be picked up and used. This should be changed such that a ConversionService specified in the ApplicationContext will be used, otherwise the default will be.

[odrotbohm]

Can you elaborate on the context of this request? The sole purpose of using a ConversionService is to use the default handlings of custom to-string conversions.

[nebhale]

Well, you asked me to open it, so I'm hoping you remember why. I believe it had something to do with what happens when a user registers a custom Converter or ConversionService in the ApplicationContext. At the moment, that custom instance is not used, only the default is. Let me poke about in our chat history to see if I can find something more.

[nebhale]

[Nov-11 19:58] Ben Hale: If you've got a minute, I've got a problem using methodOn().
[Nov-11 19:58] Oliver Gierke: go ahead
[Nov-11 19:58] Ben Hale: The methods that I'm using take a rich type and I'm using the SpringDataJPA support for converting directly to the type from the @PathVariale.
[Nov-11 19:59] Ben Hale: When methodOn() runs though, I get @org.springframework.web.bind.annotation.PathVariable com.nebhale.devoxx2013.domain.Game to type java.lang.String
[Nov-11 19:59] Ben Hale: Which makes sense.  It's not a string, so it's going to be difficult to convert back to the integer.

[Nov-11 19:59] Ben Hale: Is there something to do to hint it?
[Nov-11 20:01] Oliver Gierke: Not really unfortunately. The issue here is with the conversion being applied on the binding. We deploy a Converter String -> DomainType but there's hardly a way to torn it back into an ID string automatically
        buuuut…
[Nov-11 20:03] Oliver Gierke: As far as I can see we use a conversion service, so if you register a Converter DomainType -> String, it should be invoked
[Nov-11 20:03] Ben Hale: Yeah, that's what I was thinking.  I'll give that a go.  Thanks for your time.
[Nov-11 20:04] Oliver Gierke: doh, it's a default instance, so nothing picked up from the configuration :(
[Nov-11 20:04] Ben Hale: That's doesn't detect beans of type Converter?
[Nov-11 20:04] Oliver Gierke: nope, it's not tied to the config at all… but a static instance in BoundMethodParameter
[Nov-11 20:06] Ben Hale: OK, then I'll have to do things a bit more manually that I'd like, but it'll get good enough for the demo.  Are you going to be at Devoxx?
[Nov-11 20:06] Oliver Gierke: Nope, flying out to London for Spring eXchange on Wednesday.
        Would you mind creating a ticket for that in the GitHub tracker?
[Nov-11 20:07] Ben Hale: OK, no worries.  Have a good trip and thanks for this last-minute help.
        Yeah, I'll take care of that this evening.
[Nov-11 20:07] Oliver Gierke: No need to hurry, just want to make sure we don't loose track :) Have fun at Devoxx! :)

[Eviradnus]

+1
To give an example, here is my use case, which is pretty common since it's using spring-data components.

I'm using spring-data's DomainClassConverter (e.g. via @EnableSpringDataWebSupport, as explained in the documentation) and I have a @Controller like this one:

@Controller
@RequestMapping("/api/users")
@ExposesResourceFor(User.class)
public class UserApiController {
  @Autowired UserAssembler userAssembler;
  @RequestMapping("/{id}")
  public ResponseEntity<User> get(@PathVariable("id") User user) {
        return new ResponseEntity<>(userAssembler.toResource(user), HttpStatus.OK);
  }
}

Now, when generating a link to a given user entity, I do something like this:

linkTo(methodOn(UserApiController.class).get(user))

which gives me a link like /api/users/com.company.User@83415e7e.

Indeed, the path variable's string representation is statically resolved with the FallbackObjectToStringConverter (extract from AnnotatedParametersParameterAccessor.BoundMethodParameter in version 0.8.0.RELEASE):

        public String asString() {

            if (value == null) {
                return null;
            }

            return (String) CONVERSION_SERVICE.convert(value, parameterTypeDecsriptor, STRING_DESCRIPTOR);
        }

I think it makes sense to make CONVERSION_SERVICE configurable (or extensible), so that one may register a converter returning the ID of entities, instead of the quite useless (yet workaround-providing) toString()-based fallback. Without this extension point, only"simple" types may be bound in controller methods...

I hope it all makes sense. :)
I may try to provide a pull request if you're lacking time to solve this (though I'm not sure how you'd gracefully get Spring beans from these static mechanisms).
Best regards.

[edrik]

+1
duplicates #149, #144

[Bram80]

Any idea when this issue will be handled?
For the time being, I followed the solution described here: http://stackoverflow.com/a/22263269/2054927

Ugly, but it doesn't block development while still taking advantage of Spring Hatoas...

[Gotusso]

+1

[agibalov]

+1

[odrotbohm]

We're gonna do three things:

• detect Identifiable being implemented by the type handled to invoke getId() before the conversion
• allow to register a custom ConversionService on the ControllerLinkBuilderFactory to customize the conversion in case the LinkBuilder instances are obtained through the factory
• fall back to use the toString() method of the object at hand as a last resort

[ghost]

+1

[bostond]

+1

[hobo05]

+1

[ismailbay]

+1

[tufelix]

+1

[francesc79]

+1

[osvaldopina]

I developed a framework to render links in spring hateoas projects. It also supports rendering links for user defined types. An explanation can be found in this stackoverflow answer

[snowe2010]

Is there an end in sight for this bug? It's been open for almost 2 years and none of the provided solutions have worked in our situation.

[kkondratov]

+1

[aksdb]

+1

[gregturn]

Need to figure out a pared back version of this. The issue is against Spring Data JPA + Spring Data REST, so it's current incarnation requires hooking together lots of components. If there is a Spring HATEOAS-specific issue, need a much simpler case to illustrate.

[rgacki]

A simple case:

I have controller method parameter with custom types that are annotated with annotations such as @PathVariable or @RequestParam. While Spring MVC is behaving correctly by using my custom org.springframework.core.convert.converter.Converters, spring-hateoas is not using these converters when Links to the methods are built with org.springframework.hateoas.mvc.ControllerLinkBuilder and values are passed to the proxied method call.

A (simplified) code example:

@Controller
@RequestMapping("/users/{userId}")
public class UserProfileController {

    @GetMapping(produces = MediaType.TEXT_HTML_VALUE)
    public ModelAndView showProfile(final @PathVariable("userId") String userId,
                                    final @RequestParam(required = false) UriTemplate customVariable) {
        final Map<String, Object> model = new HashMap<>();
        final URI action = ControllerLinkBuilder.linkTo(ControllerLinkBuilder.methodOn(UserProfileController.class).updateProfile(userId, customVariable, null))
                .toUri();
        model.put("formAction", action);
        return new ModelAndView("template", model);
    }

    @PostMapping(consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE, produces = MediaType.TEXT_HTML_VALUE)
    public ModelAndView updateProfile(final @PathVariable("userId") String userId,
                                      final @RequestParam(required = false) UriTemplate customVariable,
                                      final @Valid @ModelAttribute UserForm form) {
        ....
    }

}

Without my hack, creating the form action URI would result in a ConverterNotFoundException thrown by the ConversionService from within the AnnotatedParametersParameterAccessor.

I am not using Spring Data / Spring Data REST.

[gregturn]

Spring HATEOAS at its core has a static conversion service setup to support the static factory methods. The related commit is a first swing at injecting an override. I wanted to discuss it with Oliver since many things are starting to come out as consequences of these static helpers.

[caoyuanqi]

Just post a temporary Config Fix in Spring boot, with help from https://stackoverflow.com/questions/22240155/converter-from-pathvariable-domainobject-to-string-using-controllerlinkbuilde/22263269#22263269

/**
 * This configuration file is to fix bug of Spring Hateoas.
 * please check https://github.com/spring-projects/spring-hateoas/issues/118.
 */

@Component
public class MvcConfig extends WebMvcConfigurerAdapter {

    @Autowired
    private ApplicationContext applicationContext;

    @Override
    public void addFormatters(final FormatterRegistry registry) {
        super.addFormatters(registry);

        try {
            Class<?> clazz = Class.forName("org.springframework.hateoas.mvc."
                    + "AnnotatedParametersParameterAccessor$BoundMethodParameter");
            Field field = clazz.getDeclaredField("CONVERSION_SERVICE");
            field.setAccessible(true);
            DefaultFormattingConversionService service =
                    (DefaultFormattingConversionService) field.get(null);
            for (Formatter<?> formatter : applicationContext
                    .getBeansOfType(Formatter.class).values()) {
                service.addFormatter(formatter);
            }
            for (Converter<?, ?> converter : applicationContext
                    .getBeansOfType(Converter.class).values()) {
                service.addConverter(converter);
            }
        } catch (Exception ex) {
            throw new RuntimeException(ex);
        }
    }
}

[gregturn]

@caoyuanqi While you're doing that reflection magic, you may find Spring Framework's ReflectionUtils and ReflectionTestUtils something of interest to make interacting with Java's Reflection API a little easier to use.

[laszlocsontos]

I ran into this issue yesterday and managed to work it around this way.

import static java.lang.reflect.Modifier.FINAL;
import static org.springframework.util.ReflectionUtils.findField;
import static org.springframework.util.ReflectionUtils.makeAccessible;
import static org.springframework.util.ReflectionUtils.setField;
...
@Slf4j
@Configuration
public class WebMvcConfig implements WebMvcConfigurer {

    private static final String APPA$BMP_CLASS =
            "org.springframework.hateoas.mvc.AnnotatedParametersParameterAccessor$BoundMethodParameter";

    private static final String APPA$BMP_CONVERSION_SERVICE = "CONVERSION_SERVICE";

    @Override
    public void addFormatters(FormatterRegistry registry) {
        ...
        workaround((ConversionService) registry);
    }

    /*
     * Workaround for https://github.com/spring-projects/spring-hateoas/issues/118
     */
    private void workaround(ConversionService conversionService) {
        try {
            ReflectionUtils.doWithFields(
                    Class.forName(APPA$BMP_CLASS),
                    it -> setValue(it, conversionService),
                    it -> APPA$BMP_CONVERSION_SERVICE.equals(it.getName())
            );
        } catch (ClassNotFoundException e) {
            log.error(e.getMessage(), e);
        }
    }

    private void setValue(Field field, Object value) {
        // Remove final modifier
        Field modifiersField = findField(Field.class, "modifiers");
        makeAccessible(modifiersField);
        setField(modifiersField, field, field.getModifiers() & ~FINAL);

        // Set field value
        makeAccessible(field);
        setField(field, null, value);
    }

}

I can also see that there has been PR (#618) out by @gregturn for some time.

Is there a chance that it's going to get reviewed and merged anytime soon?

[jannikweichert]

Any updates on this?

[matthenry87]

+1

[Zachery-Harley]

+1

[hdpe]

@laszlocsontos's solution above still works well for 1.0.3 with the change:

private static final String APPA$BMP_CLASS =
        "org.springframework.hateoas.server.core.WebHandler$HandlerMethodParameter";

I guess ...BMP_CLASS could also do with renaming.

Thanks

[aisensiy]

@laszlocsontos's solution above still works well for 1.0.3 with the change:

private static final String APPA$BMP_CLASS =
        "org.springframework.hateoas.server.core.WebHandler$HandlerMethodParameter";

I guess ...BMP_CLASS could also do with renaming.

Thanks

It seems not working with hateoas:0.24.0. But maybe it is my mistake. I have a converter like this:

@Component
public class StringToJodaTimeConverter implements Converter<String, DateTime> {

  @Override
  public DateTime convert(@NotNull String source) {
    return DateTime.parse(source, DateTimeFormat.forPattern("YYYY-MM-dd HH:mm:ss").withZoneUTC());
  }
}

And the method signature looks like this:

public ResponseEntity getRequests(
      @RequestParam(value = "to", required = false) DateTime upTo,
      @AuthenticationPrincipal User current) {

I am trying to convert the DateTime upTo field. The hateoas used like this:

linkTo(methodOn(ServingVersionRequestsApi.class).getRequests(DateTime.now(UTC), null)).withRel("next")

[eirikaho]

Issue not resolved in 1.0.4. @laszlocsontos solution above with @aisensiy modification is necessary still.

[Eric-Jalal]

Can someone please clarify what is the situation with this problem? since this solution does not even work anymore since there is no more org.springframework.hateoas.mvc.AnnotatedParametersParameterAccessor .

[eirikaho]

Can someone please clarify what is the situation with this problem? since this solution does not even work anymore since there is no more org.springframework.hateoas.mvc.AnnotatedParametersParameterAccessor .

I use the solution from @laszlocsontos with @aisensiy modification with spring HATEOAS 1.1.2.

TL:DR;
change org.springframework.hateoas.mvc.AnnotatedParametersParameterAccessor$BoundMethodParameter to org.springframework.hateoas.server.core.WebHandler$HandlerMethodParameter

[odrotbohm]

This is in place now. We look up the ConversionService from the ApplicationContext associated with the current request and fall back to a default instance of none available. Feel free to give the 1.3 snapshots a try.