Spring Framework
  1. Spring Framework
  2. SPR-7608

@PathVariable and @ModelAttribute incompatibility prevent me from having a nice "update" handler

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.1 RC1
    • Component/s: Web
    • Labels:
      None
    • Last commented by a User:
      false

      Description

      Spring MVC examples often refer to the creation of an entity using @ModelAttribute.

      A common pattern is to update an existing entity.
      Generally you want to load the existing entity and then apply on it the allowed fields.

      In my controller I have an handler for showing an entity, one for creating a new one...
      here are their signature:

      @RequestMapping("show/

      {pk}")
      public String show(@PathVariable("pk") Account account, Model model)

      @RequestMapping(value = "create", method = RequestMethod.POST)
      public String create(@Valid Account account, BindingResult bindingResult, Model model)


      Now, of course I need a handler, as nice as the one above to update the entity.
      Here it is:

      @RequestMapping(value = "update/{pk}

      ", method =

      { RequestMethod.PUT, RequestMethod.POST }

      )
      public String update(@Valid @PathVariable("pk") Account account, BindingResult bindingResult, Model model);

      The handler above is not accepted by SPring MVC, however I think it should! Here is what I would expect:
      1/ convert the pk to the Account entity using the corresponding Parser (it works for "show" handler above...)
      2/ Once converted, since the account is placed just before the bindingResult, it should be used to bind the request parameters that is to update directly the entity.
      3/Once updated, it should perform the validation.

      Note: I am confident in updating directly the entity as I know I can restrict the allowed fields thanks to an InitBinder.

      The code above throw an exception:
      org.springframework.web.bind.annotation.support.HandlerMethodInvocationException: Failed to invoke handler method [public java.lang.String fr.nnn.web.controller.AccountController.update(fr.nnn.domain.Account,org.springframework.validation.BindingResult,java.lang.Boolean,java.lang.Boolean,org.springframework.ui.Model)]; nested exception is java.lang.IllegalStateException: Errors/BindingResult argument declared without preceding model attribute. Check your handler method signature!

      I tried to add the @ModelAttribute("account") annotation after @PathVariable("pk") but then another exception is thrown... stating that I cannot use both annotations at the same time.

      Am I missing something or do you agree it would be a nice feature?

        Issue Links

          Activity

          Hide
          Rossen Stoyanchev added a comment -

          However, we would like to encapsulate all CRUD methods of an entity into a single controller.

          I think there may be a way to resolve this cleanly with the new RequestMappingHandlerAdapter in Spring 3.1, which resolves each method argument with a separate HandlerMethodArgumentResolver. We could enhance the PathVariableMethodArgumentResolver to apply data binding if (a) the argument is not a simple type (int, String, etc) and (b) the next argument is BindingResult.

          Note also that in 3.1 an @PathVariable is automatically added to the model (SPR-7543).

          Show
          Rossen Stoyanchev added a comment - However, we would like to encapsulate all CRUD methods of an entity into a single controller. I think there may be a way to resolve this cleanly with the new RequestMappingHandlerAdapter in Spring 3.1, which resolves each method argument with a separate HandlerMethodArgumentResolver. We could enhance the PathVariableMethodArgumentResolver to apply data binding if (a) the argument is not a simple type (int, String, etc) and (b) the next argument is BindingResult. Note also that in 3.1 an @PathVariable is automatically added to the model ( SPR-7543 ).
          Hide
          Árpád Tamási added a comment -

          I think there may be a way to resolve this cleanly ...

          Great. I gladly contribute in any way.

          Note also that in 3.1 an @PathVariable is automatically added to the model (SPR-7543).

          It's comfortable usually but sometimes we don't need it in the model. E.g. POST requests usually end up with redirection. I'm sure there are other situations also so it would be nice to have a way to disable this functionality.

          Show
          Árpád Tamási added a comment - I think there may be a way to resolve this cleanly ... Great. I gladly contribute in any way. Note also that in 3.1 an @PathVariable is automatically added to the model ( SPR-7543 ). It's comfortable usually but sometimes we don't need it in the model. E.g. POST requests usually end up with redirection. I'm sure there are other situations also so it would be nice to have a way to disable this functionality.
          Hide
          Rossen Stoyanchev added a comment -

          It's comfortable usually but sometimes we don't need it in the model. E.g. POST requests usually end up with redirection. I'm sure there are other situations also so it would be nice to have a way to disable this functionality.

          @PathVariables are never appended automatically to the query string (SPR-8448).

          Show
          Rossen Stoyanchev added a comment - It's comfortable usually but sometimes we don't need it in the model. E.g. POST requests usually end up with redirection. I'm sure there are other situations also so it would be nice to have a way to disable this functionality. @PathVariables are never appended automatically to the query string ( SPR-8448 ).
          Hide
          Árpád Tamási added a comment -

          That's great but sometimes we do not need them in the returned model. Neither in the query string nor in JSTL expressions. Redirection was just an example of these cases. Anyway, it's not a big issue.

          Show
          Árpád Tamási added a comment - That's great but sometimes we do not need them in the returned model. Neither in the query string nor in JSTL expressions. Redirection was just an example of these cases. Anyway, it's not a big issue.
          Hide
          Rossen Stoyanchev added a comment - - edited

          I've checked in the proposed solution. The approach is based on the @ModelAttribute annotation. Essentially it becomes one of the model attribute instantiation strategies. The order of instantiation becomes:

          1. Is it in the session (via @SessionAttributes)?
          2. Is it in the model (via @ModelAttribute method)?
          3. Does the model attribute match to a URI template var by name and is type conversion possible?
          4. Default constructor instantiation

          When created from a URI template variable, an attempt is made to locate and use a registered Converter or PropertyEditor. This also includes checking and using a single String-argument constructor if present on the target class. That's just a feature of Spring MVC's type conversion.

          If you do give this a try, which would be great, please keep in mind you need to use the new RequestMappingHandlerMapping and RequestMappingHandlerAdapter. With the MVC namespace that will be the case, or otherwise just replace DefaultAnnotationHandlerMapping and AnnotationMethodHandlerAdapter with the above.

          Show
          Rossen Stoyanchev added a comment - - edited I've checked in the proposed solution. The approach is based on the @ModelAttribute annotation. Essentially it becomes one of the model attribute instantiation strategies. The order of instantiation becomes: Is it in the session (via @SessionAttributes)? Is it in the model (via @ModelAttribute method)? Does the model attribute match to a URI template var by name and is type conversion possible? Default constructor instantiation When created from a URI template variable, an attempt is made to locate and use a registered Converter or PropertyEditor . This also includes checking and using a single String-argument constructor if present on the target class. That's just a feature of Spring MVC's type conversion. If you do give this a try, which would be great, please keep in mind you need to use the new RequestMappingHandlerMapping and RequestMappingHandlerAdapter. With the MVC namespace that will be the case, or otherwise just replace DefaultAnnotationHandlerMapping and AnnotationMethodHandlerAdapter with the above.

            People

            • Assignee:
              Rossen Stoyanchev
              Reporter:
              Nicolas Romanetti
              Last updater:
              Trevor Marshall
            • Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                2 years, 43 weeks, 5 days ago