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

Add @FormAttribute attributes to customize x-www-form-urlencoded [SPR-13433] #18012

Closed
spring-projects-issues opened this issue Sep 4, 2015 · 45 comments
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Sep 4, 2015

Phil Webb opened SPR-13433 and commented

As requested on the Spring Boot issue tracker:
spring-projects/spring-boot#3890

When processing an HttpRequest with x-www-form-urlencoded content, we can use a controller with POJO matching the payload structure.

Example payload: value_first=value1&value_second=value2

@RequestMapping(method = RequestMethod.POST, consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE)
 public String handlePost(@ModelAttribute Body body) {
 }

POJO:

public class Body {
 private String value_first;
 private String value_second;
}

The problem is that the variable names must match the field names in the HttpRequest payload in order to be processed by the controller. There is not way of naming them differently, for example, if I do not want to use underscores in Java variable and method names.

What I would appreciate would be using something like this:

@FormAttribute
private String value1;

Issue Links:

20 votes, 16 watchers

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 8, 2015

Rossen Stoyanchev commented

Adding link to an (old) related ticket #13880.

Wouldn't that have to be @FormAttribute("value_first")? I'm wondering what's the idea. For such a cross-cutting requirement such as using underscores vs camelback notation, putting @FormAttribute on every field seems repetitive.

@spring-projects-issues
Copy link
Collaborator Author

Martin Myslík commented

I should have clarified this in my simplified example. It should , of course, be:

@FormAttribute("value_first")
private String value1;

As for the requirement, I am not saying that this is a core functionality of Spring that I am desperately missing but imagine this scenario:

You are processing a form-encoded payload from a third party service and you want to use POJO to map the values. You use camelCase in your whole project and now you are forced to use underscopes if the third party service does so (in the POJO) or write your own converter just for this purpose.

When I encountered this problem, I immediately looked for some Spring functionality to tackle this and was surprised that there is none. Feel free to close this issue if you feel that I am being unreasonable but I expected more people having this issue as well.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I think the scenario is quite clear. There is nothing unreasonable about your request. I'm only wondering whether annotations would solve this effectively. An annotation here and there to customize a field name is okay but having to put one on every field to adapt to different naming strategy -- that feels more like something that should be more centralized, otherwise you'd have to have one on every field.

If the use case is purely input based, i.e. data binding from a request such as a REST service, then a Filter could wraps the request and overrides the getRequestParam() and related method to check for both "firstValue" and also "first_value".

@spring-projects-issues
Copy link
Collaborator Author

Martin Myslík commented

I had several use cases where I had to extract just several fields from a huge form-encoded request which would mean putting this annotation on a couple of properties but you are right that you still have to annotate every property of such POJO. Perhaps some higher level annotation for the whole classto automatically convert underscopes in the field names to camel case or a filter as you are suggesting would be more elegant solution.

@spring-projects-issues
Copy link
Collaborator Author

Micah Silverman commented

Bringing this up again. ;) There are still a number of important providers that insist on x-www-form-urlencoded when POSTing. Regarding the concern around lots of annotations on a POJO, @JsonProperty has worked very well for application/json type POSTs. It makes everything automatic, even if the result is every field having an annotation on it.

I recently encountered this directly in working with Slack's slash command features. https://api.slack.com/slash-commands. In short, you register an endpoint with Slack, and when you issue a "slash" command, slack POSTs to your endpoint with x-www-form-urlencoded Content-type.

So, one approach to handle that would look like this:

@RequestMapping(
  value = "/slack",
  method = RequestMethod.POST,
  consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE
)
public MyResponse onReceiveSlashCommand(
  @RequestParam("token") String token,
  @RequestParam("team_id") String teamId,
  @RequestParam("team_domain") String teamDomain,
  @RequestParam("channel_id") String channelId,
  @RequestParam("channel_name") String channelName,
  @RequestParam("user_id") String userId,
  @RequestParam("user_name") String userName,
  @RequestParam("command") String command,
  @RequestParam("text") String text,
  @RequestParam("response_url") String responseUrl
) {
    ...
}

This is pretty gnarly, especially in the age of Spring boot's built in Jackson mapper handling.

So, I set out to do this:

public class SlackSlashCommand {

    private String token;
    private String command;
    private String text;

    @JsonProperty("team_id")
    private String teamId;

    @JsonProperty("team_domain")
    private String teamDomain;

    @JsonProperty("channel_id")
    private String channelId;

    @JsonProperty("channel_name")
    private String channelName;

    @JsonProperty("user_id")
    private String userId;

    @JsonProperty("user_name")
    private String userName;

    @JsonProperty("response_url")
    private String responseUrl;

    ...
}

If the POST were sent as application/json, then the controller would look like this and we'd be done:

    @RequestMapping(value = "/slack", method = RequestMethod.POST)
    public @ResponseBody SlackSlashCommand slack(@RequestBody SlackSlashCommand slackSlashCommand) {
        log.info("slackSlashCommand: {}", slackSlashCommand);

        return slackSlashCommand;
    }

But, slack will only POST with x-www-form-urlencoded. So, I had to make the controller method like this:

    @RequestMapping(
        value = "/slack", method = RequestMethod.POST,
        consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE, produces = MediaType.APPLICATION_JSON_VALUE
    )
    public @ResponseBody SlackSlashCommand slack(SlackSlashCommand slackSlashCommand) {
        log.info("slackSlashCommand: {}", slackSlashCommand);

        return slackSlashCommand;
    }

Only problem is that the underscore properties coming in from Slack get ignored when materializing the SlackSlashCommand. If there were an analog to @JsonProperty for form POSTs, then this would be handled automatically.

What I did for now to get around this, and so as to not pollute my SlackSlashCommand class, is a little ugly, but it works:

// workaround for customize x-www-form-urlencoded
public abstract class AbstractFormSlackSlashCommand {

    public void setTeam_id(String teamId) {
        setTeamId(teamId);
    }

    public void setTeam_domain(String teamDomain) {
        setTeamDomain(teamDomain);
    }

    public void setChannel_id(String channelId) {
        setChannelId(channelId);
    }

    public void setChannel_name(String channelName) {
        setChannelName(channelName);
    }

    public void setUser_id(String userId) {
        setUserId(userId);
    }

    public void setUser_name(String userName) {
        setUserName(userName);
    }

    public void setResponse_url(String responseUrl) {
        setResponseUrl(responseUrl);
    }

    abstract void setTeamId(String teamId);
    abstract void setTeamDomain(String teamDomain);
    abstract void setChannelId(String channelId);
    abstract void setChannelName(String channelName);
    abstract void setUserId(String userId);
    abstract void setUserName(String userName);
    abstract void setResponseUrl(String responseUrl);
}

public class SlackSlashCommand extends AbstractFormSlackSlashCommand {
...
}

That's a lot of boilerplate to accomplish what Jackson can do automatically with the @JsonProperty annotation! In fact, I left those annotations in so that I can receive the SlackSlashCommand object in the controller and return it as a @ResponseBody:

http -v -f POST localhost:8080/api/v1/slack2 token=token team_id=team_id team_domain=team_domain channel_id=channel_id channel_name=channel_name user_id=user_id user_name=user_name command=command text=text response_url=response_url
POST /api/v1/slack2 HTTP/1.1
Accept: */*
Content-Type: application/x-www-form-urlencoded; charset=utf-8
...

token=token&team_id=team_id&team_domain=team_domain&channel_id=channel_id&channel_name=channel_name&user_id=user_id&user_name=user_name&command=command&text=text&response_url=response_url

HTTP/1.1 200
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Content-Type: application/json;charset=UTF-8
Date: Mon, 22 May 2017 05:28:27 GMT
...
{
    "channel_id": "channel_id",
    "channel_name": "channel_name",
    "command": "command",
    "response_url": "response_url",
    "team_domain": "team_domain",
    "team_id": "team_id",
    "text": "text",
    "token": "token",
    "user_id": "user_id",
    "user_name": "user_name"
}

@spring-projects-issues
Copy link
Collaborator Author

Micah Silverman commented

Two approaches to solve the problem. Both require more boilerplate than is necessary for accomplishing the same thing with JSON:

https://gist.github.com/dogeared/7e60a2caebd00c959f0d7e24ef79b54e
https://gist.github.com/dogeared/0db806ad56258711de0ffdfa5317ee42

This first approach uses an abstract super-class that breaks Java naming conventions. Pros: subclass is kept "clean". It's clear what's going on in the super class and why. No additional converters or configuration needed. Cons: Breaks Java naming conventions.

The second approach is the more "proper" approach. It uses an HttpMessageConverter. Pros: it's more idiomatic Spring and doesn't break any naming conventions. It enables using @RequestBody annotation, as a Spring developer would expect. Cons: more "manual labor" involved in building the POJO by hand.

@spring-projects-issues
Copy link
Collaborator Author

Micah Silverman commented

I created a gist for a third approach: https://gist.github.com/dogeared/3ebb46b9d948c023e702ccb9ecfdf35e

@spring-projects-issues
Copy link
Collaborator Author

Micah Silverman commented

I also created a blog post on the subject ;) https://afitnerd.com/2017/05/24/what-if-spring-boot-handled-forms-like-json/

@spring-projects-issues

This comment was marked as duplicate.

@spring-projects-issues

This comment was marked as duplicate.

@spring-projects-issues

This comment was marked as duplicate.

@spring-projects-issues
Copy link
Collaborator Author

Zhang Jie commented

Hi, all, I have found that, if we want to use custom @FormAttribute or something else to customize x-www-form-urlencoded, we can write a custom FormAttributeBeanInfoFactory which is similar to ExtendedBeanInfoFactory, and which can create FormAttributeBeanInfo similar to ExtendedBeanInfo and using @FormAttribute to get property name(by propertyNameFor()).
When we configure FormAttributeBeanInfoFactory into /META-INF/spring.factories with key org.springframework.beans.BeanInfoFactory, it will be used by WebDataBinder and BeanWrapperImpl, and will work as expected with @ModelAttribute.
I didn't test all test case, but i think it will be an alternative way to custom parameter name when binding an object.

@kang0921ok
Copy link

Hi I'm newbie ^^;
I hava a question.
Is that development over?

@sbrannen
Copy link
Member

sbrannen commented Sep 2, 2019

Is that development over?

This issue is still "Open".

@nuno-aj-aniceto
Copy link

This would be a nice improvement & boost to the framework.

@HongyiFu
Copy link

No one mentioned hyphens? I feel it's not all that uncommon for a query param to be "team-id". Without this feature, it seems impossible to do this now?

@xfan-shop
Copy link

I have made a simple example in my project for form data according to Zhang Jie 's suggestion.
https://stackoverflow.com/questions/38171022/how-to-map-multiple-parameter-names-to-pojo-when-binding-spring-mvc-command-obje/57217544#57217544

@mattbertolini
Copy link
Contributor

I was encountering similar issues with query params and form data having different formats to my @ModelAttribute property names. I have attempted to solve my problem in a generic Spring-like way. Here is my attempt: https://github.com/mattbertolini/spring-annotated-web-data-binder. Posting here in case someone finds it useful.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Apr 30, 2020

Thanks @mattbertolini. Two things I'm wondering about.

One is nested binding. Taking the Slack commands example from above with keys like "channel_name", it seems unlikely that keys like "channel.channel_name" that match to a nested object structure would come into play. In one of the approaches from @dogeared (i.e. converting the form data to a Map, and then using Jackson to map to a higher level Object via JSON) I don't expect that nested bindings would work. I do see that your solution does support nested bindings but is that something you expect as a valid case or is this primarily for binding to a flat object?

Two is the other side of parsing, i.e. formatting and displaying bound values and errors. Would you expect that a BindingResult would respect and use the original aliases from which values were bound. The BindingResult can be used to report errors (e.g. with BindingResultException) or it could be used on an HTML form to display the form for the user to correct errors. Or maybe the primary use case for this feature isn't form POSTs within the same HTML/browser application, but rather support for accepting external submissions of form data (i.e. in a @RestController) which are outside an application's control?

@mattbertolini
Copy link
Contributor

Hi @rstoyanchev. Thanks for the questions.

Re: nested binding: You are correct that in the slack commands example nested binding would be unnecessary. My vision for nested bindings was mainly for grouping together similarly linked parameters. More of a logical thing for the engineer than for modeling an external API. I mostly expect the library to be used for flat objects and nested binding to be a niche case. Since I was modeling my library after JAX-RS *Param annotations, I added nested binding in as well. I'm definitely looking for additional use cases for nested binding to highlight in my documentation so if anyone has some I'd love to hear about them.

Re: formatting and errors. You are correct that the BindingResult would lose the original aliases and only be aware of the bean property names. That can be confusing when reporting errors from and HTML form. I have worked around this limitation by compensating with more detailed error messages to help the user or engineer. It's not the best solution but it was an acceptable trade-off given the benefits I was getting in return.

I mostly use my approach for API development (@RestController) rather than form POSTs inside of browser applications. One of my design goals was to not modify the default behavior of Spring MVC so that the existing @ModelAttribute would still work as expected. That way I could mix and match depending on the use case I was encountering. I still use @ModelAttribute for a good portion of my web forms as I have control over the HTML as well.

I hope this answers your questions. Thanks for taking a look at my library. Much appreciated.

@fhansen87
Copy link

+1

@igorcavalcanti
Copy link

igorcavalcanti commented Sep 28, 2020

I did this:

Controller:

@PostMapping(value = {"/status"}, consumes = MediaType.APPLICATION_FORM_URLENCODED)
public ResponseEntity<String> statusWarning(Body request) {
         return ResponseEntity.ok("OK");
}

POJO:

public class Body {
    private String value1;
    private String value2;

    public Body (String value_first, String value_second) {
        value1 = value_first;
        value2 = value_second;
    }
}

@puertajennifer
Copy link

I found this article: https://www.metakoder.com/blog/parse-snake-case-json-in-spring-boot/ I hope this help you as much as it did it to me!

@puertajennifer
Copy link

I my case, the Json I was receving it's in UpperCamelCaseStrategy, so I added this sentence just above the class definition of the POJO I´m receiving with @RequestBody

@JsonNaming(PropertyNamingStrategy.UpperCamelCaseStrategy.class)
public class MyClass{
}

@gavenkoa
Copy link

gavenkoa commented Feb 6, 2022

@puertajennifer @JsonNaming has nothing with POST form + x-www-form-urlencoded. Jackson is not used when you submit a form.

@gavenkoa
Copy link

gavenkoa commented Feb 6, 2022

Related questions with workarounds:

I don't like OncePerRequestFilter which is global, not per mapping. Or custom ExtendedServletRequestDataBinder to accept new kind of annotation on form parameters...

@Nasil
Copy link

Nasil commented Jul 11, 2022

I did this:

Controller:

@RequestMapping(value = "/slack", method = RequestMethod.POST, consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
public @ResponseBody SlackSlashCommand slack(@RequestBody MultiValueMap<String, Object> reqMap) {
      objectMapper.enable(DeserializationFeature.UNWRAP_ROOT_VALUE);
      objectMapper.enable(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS);
      SlackSlashCommand slackSlashCommand = objectMapper.convertValue(reqMap, SlackSlashCommand.class);
      return slackSlashCommand;
}

Just use @JsonProperty with underscore.

public class SlackSlashCommand {

    @JsonProperty("team_id")
    private Long teamId; // Type long 

    @JsonProperty("team_domain")
    private String teamDomain; // Type String

    @JsonProperty("insert_timestamp")
    private LocalDateTime insertTimestamp; // Type LocalDateTime
}

@CharlelieBouvier

This comment was marked as duplicate.

@gavenkoa
Copy link

@Nasil I'm not sure you have to create and configure ObjectMapper each time in the controller. This class was designed to be thread safe, from JavaDoc:

Mapper instances are fully thread-safe

You have to declare it as a bean ))

I wonder if the "hack" will work on complex objects instead of String, when you attempt to resolve MultiValueMap<String, String> to a class with LocalDateTime or BigDecimal setters.

@Ynnck123

This comment was marked as duplicate.

1 similar comment
@alex-semenk

This comment was marked as duplicate.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jul 26, 2023

For 6.1, we've added enhanced support for constructor binding in DataBinder including nested constructors (#20806) the option to customize the bind value name with an @BindParam annotation (#30947). For example:

public class SlackSlashCommand {

    private String token;
    private String command;
    private String text;
    private String teamId;
    private String teamDomain;

    public SlackSlashCommand(
            String token, String command, String text, 
            @BindParam("team_id") String teamId, @BindParam("team_domain") String teamDomain) {

        // ...
    }

}

In light of this, I am closing this issue as superseded.

@rstoyanchev rstoyanchev closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2023
@rstoyanchev rstoyanchev removed this from the General Backlog milestone Jul 26, 2023
@rstoyanchev rstoyanchev added the status: superseded An issue that has been superseded by another label Jul 26, 2023
@gavenkoa
Copy link

gavenkoa commented Jul 26, 2023

@rstoyanchev Could you clarify the solution?

I have a @Controller with many @RequestMapping inside. Each method has an attribute marked as @Model. Should I mark arguments of a single all args constructor for a model with url-encoded names in @BindParam?

Like for your example if I write:

@PostRequest(path = "/something")
Object process(@Model SlackSlashCommand command) { ... }

for my POST form with x-www-form-urlencoded Spring MVC will set command.teamId to team_id from a form?

@rstoyanchev
Copy link
Contributor

@gavenkoa, yes that's how it is expected to work. Just minor note that it's @ModelAttribute rather than @Model.

@nkavian
Copy link

nkavian commented Aug 1, 2023

Does this solution support binding individual fields? Using / forcing a constructor is not ideal. For example, an object that uses Lombok, Jackson, or requires other annotations, versus the stated solution would funnel the fields through a constructor. I know they are slightly different concerns but it makes it harder to get them to work together.

@Data
public class Account {

    @BindParam("account_id") <--- Is this supported
    @JsonProperty("account_id")
    private String accountId;

}

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Aug 2, 2023

A constructor is required and I think @Data gives you that. The annotation can be on a field as long as its name matches the constructor parameter. @BindParam is supported out of the box, but you can plug in a NameResolver on DataBinder that looks for a different annotation such as @JsonProperty. This is the resolver for @BindParam for reference.

@nkavian
Copy link

nkavian commented Aug 3, 2023

Thanks for replying. @Data does not give you a constructor you can annotate. The point of using Lombok was to write less boilerplate code. Lombok does have an @AllArgsConstructor but again that hides the actual boilerplate.

While @BindParam is a solution; it doesn't feel like it is the solution.

  • Not using the @BindParam solution requires less code.
  • Less code implies it's cleaner and potentially more safer to stick with.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Aug 3, 2023

What I said is that you do not need to annotate the constructor, and also that you do not need to use @BindParam. My expectation is that your Account object should not have to change. You'll just need to register a NameResolver to find the name from @JsonProperty.

@nkavian
Copy link

nkavian commented Aug 3, 2023

A constructor is required and I think @DaTa gives you that.

Lombok doesn't provide a constructor.

@rstoyanchev
Copy link
Contributor

Sorry, I'm no expert on Lombok. I was looking here https://projectlombok.org/features/Data.

@rstoyanchev
Copy link
Contributor

@nkavian I gave the scenario with Lombok a try. If I change accountId to a final field, Lombok exposes a constructor, and it works. I uncovered a small issue that currently @BindParam can't be put on a field, but I will fix that shortly. That said, you don't need to use @BindParam at all. If you already have @JsonProperty, you just need to set a NameResolver for it similar to BindParamNameResolver.

@nkavian
Copy link

nkavian commented Aug 3, 2023

I'll avoid try to avoid any further replies, I've already tried to point out this solution is too narrow (that requires the client to do very specific things to make it work). It would be better to see a simpler solution with fewer dependencies on how the client writes their code.

  1. Clients should not be forced to make fields final.
  2. Clients should not be forced to create constructors when their original code didn't require it.
  3. It would be better if Spring provided a builtin NameResolver to handle the mapping.
  4. The constructors that Lombok creates can not be relied upon and we explicitly avoid generating Lombok constructors. Imagine a class with 3 String fields. Lombok will create a constructor for it. If some time later a develop reorders the string fields (or removes one, and then adds another), then their old code calling the constructor will not know about the order change; and silent bugs enter into the program. From this PRs point of view, it doesn't care if the fields are moved around, it will still work; but I now have exposed my team to a constructor that I didn't want to manage which can cause me headaches.

@rstoyanchev
Copy link
Contributor

The solution doesn't require you do very specific things. It's rather meant for use with constructors and final fields, which I consider a good practice. It's fine if you prefer field injection, but we don't support that currently.

@linsmalldragon

This comment was marked as off-topic.

@fy-kenny

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests