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

Let @ModelAttribute work according with one or many specific @RequestMapping [SPR-12303] #16908

Closed
spring-projects-issues opened this issue Oct 6, 2014 · 10 comments
Assignees
Labels
in: web status: declined type: enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 6, 2014

Manuel Jordan opened SPR-12303 and commented

The link shared above is going to be used to ask for many "improvements"

Here the summary of my request: I want to know if is possible add a new annotation or perhaps a new attribute for @ModelAttribute to only be executed according of a specific @RequestMapping.

I could have many @RequestMapping's (creation GET/POST, search GET/POST, edit or update GET/POST, reports GET, JSON GET/POST, etc).

Therefore for each potential call for any of these @RequestMapping's, a possible unneccessary call to a @ModelAttribute is going to be happen

A @Controller could has 10 @RequestMapping's and few @ModelAttribute's (to create an object model and collections) and they should be executed according to only 1 or 2 @RequestMapping's.

Therefore for the rest, the others 8 @RequestMapping, the @ModelAttribute's are going to be executed unneccessarily.

I want avoid create many @Controller's


Affects: 4.0.7

Reference URL: http://stackoverflow.com/questions/25900993/spring-mvc-request-scope-trying-to-update-a-command-object-with-binder-setdisa

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 7, 2014

Rossen Stoyanchev commented

The question has come up before. The issue is there is no straight-forward, sure way to reference an @RequestMapping method. Also remember that such methods can be global (e.g. @ControllerAdvice). So I can better understand the specific scenario, could you provide something that can be executed along the lines of the code sample from the referenced in the SO thread (possibly here but doesn't have to be)? I did read through but it's quite long and raised a few question marks. It would be best to have something executable.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 9, 2014

Manuel Jordan commented

Let me create a simple sample app for this weekend and share it later.

Thanks in advance

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 11, 2014

Manuel Jordan commented

Something like this

@Controller
public class PersonController {

	@ModelAttribute("person")
	public Person createPerson()
		return new Person();
	}

	@ModelAtrribute("countries")
	public Set<Country> countries(){
   		return set of countries
	}

	@ModelAtrribute("languages")
	public Set<Languages> languages(){
		return set of languages like Spanish English Portuguese etc...
	}

	@RequestMapping(value="/person/register.htm", for GET)
	public String registerPerson(…){

	}

	@RequestMapping(value="/person/register.htm", for POST)
	public String registerPerson(…){

	}

	@RequestMapping(value="/person/update.htm", for GET)
	public String updatePerson(…){

	}

	@RequestMapping(value="/person/update.htm", for POST)
	public String updatePerson(…){

	}
	
	@RequestMapping(...)
	public String showPersons(…){
	...
	}
	
	@RequestMapping(GET: to let do a search by name, code, dates etc.. )
	public String findPerson(…){
	...
	}

	@RequestMapping(POST: to show person details)
	public String findPerson(…){ // or showDetailsPerson(...)
	...
	}
	
	@RequestMapping(value="" for Get and JSON)
	public @ResponseBody Person getPersonJson(@RequestParam ..…){
	
	}

	@RequestMapping(...)
	public @ResponseBody Set<Person> showPersonsJson( JSON ){
	...
	}
	
    //more	
    
}

Therefore we have the following @RequestMappings

  1. registerPerson(…) Get
  2. registerPerson(…) Post
  3. updatePerson(…) Get
  4. updatePerson(…) Post
  5. showPersons(…)
  6. findPerson(…) Get
  7. findPerson(…) Post - or showDetailsPerson(...)
  8. getPersonJson(…)
  9. showPersonsJson(…)

Now about the @ModelAttribute's

  • createPerson()
  • countries()
  • languages()
  1. They only should be executed perhaps for the @RequestMapping's 1,2,3,4 and 7
  2. For the rest of the cases 5,6,8,9 they should not executed.

Therefore, I want avoid create many @Controller's

I thought something like (a new annotation @ForRequestMapping)

@ModelAttribute("person")
@ForRequestMapping{values={"url1","url2","url3"}}
public Person createPerson()
	return new Person();
}

Or perhaps (new attribute)

@ModelAttribute("person", for-request-mappings={"url1","url2","url3"})
public Person createPerson()
	return new Person();
}

has sense? (Even more, other problem, what to do in case of have two URL /person/register.htm but for HTTP methods Get and Post)

If my requested improvement has no sense, what I should do to around this situation about execute unnecessarily a @ModelAttribute?

Thanks in advance

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2014

Rossen Stoyanchev commented

@ModelAttribute("person", for-request-mappings={"url1","url2","url3"})

Not an option. It would have no meaning when @ModelAttribute is used on a method parameter. The same is true when used on an @RequestMapping method where it's even more confusing as to what it means:

@RequestMapping("/whatever")
@ModelAttribute("person", for-request-mapping="huh?")

As for @ForRequestMapping{values={"url1","url2","url3"}}, it amounts to duplicating URLs and I can see further requests to add HTTP methods, etc. till we are replicating @RequestMappings. At that point is it really better than splitting controllers? In the particular example give, a split along the lines of search vs crud is not a bad option.

Outside the specific example, I do see what you're trying to do. As I said this has been requested more than once before. It's not for lack of desire, we just never had a clean way to support it. Perhaps we can do something with Java 8 method references. We just need a good annotation name.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2014

Manuel Jordan commented

About

@RequestMapping method where it's even more confusing as to what it means:

I did not write about that.

So

@RequestMapping("/whatever")
@ModelAttribute("person", for-request-mapping="huh?")

has no sense for me too.

At that point is it really better than splitting controllers? In the particular example give, a split along the lines of search vs crud is not a bad option.

Yes, but is my worst scenario. The point is keep the high in cohesion in a @Controller

It's not for lack of desire, we just never had a clean way to support it

of course, no worries, I know you all are very busy.

Perhaps we can do something with Java 8 method references. We just need a good annotation name.

We know that the default behaviour is: a @ModelAttribute is executed always when any @RequestMapping is executed. But why not think in IoC, I mean, the @RequestMapping method could call explicitly one or many @ModelAttribute, i.e:

@ModelAttribute("languages")
public Set<Language> generateLanguages(){
   ….
   return ….
}

@RequestMapping(value="createperson.htm" Get/Post)
@ExecuteModelAttribute(values={this.generateLanguages, this.method02})// needs some improvement
public String createPerson(…){
   ...
}

See the new annotation @ExecuteModelAttribute
Possible problem situation to resolve if is the generateLanguages() expects some argument/parameter

has sense my idea?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2014

Rossen Stoyanchev commented

About "@RequestMapping method where it's even more confusing as to what it means:" I did not write about that.

Right but these are the consequences of adding an attribute to @ModelAttribute. Someone else without prior knowledge of our discussion here will try to understand what the attribute means in the different places where they can be used. If it doesn't make sense in all cases it's not a nice API.

See the new annotation @ExecuteModelAttribute

The @ForRequestMapping suggestion earlier with method references (rather than URLs) would be better IMO but the issue here is exactly the same. There is no good way to refer to methods other prior to Java 8.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2014

Manuel Jordan commented

The @ForRequestMapping suggestion earlier with method references (rather than URLs) would be better IMO but the issue here is exactly the same.

Mmm, who should be agnostic?

  • The @ModelAttribute from the @RequestMapping

or

  • The @RequestMapping from the @ModelAttribute

There is no good way to refer to methods other prior to Java 8

Agree, but both know that starting from Spring 4, it uses Java 8 in peace. For prior releases Spring 3.2.x the developer would use the old school (creating many controllers).

It would be consider it for Spring 4.2 or Spring 5. In the same way when @Controller had appeared for the first time (thanks Java 5 support annotation)

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2014

Rossen Stoyanchev commented

Alright we can consider a potential @Intercept annotation with Java 8 methods references to @RequestMapping methods for 4.2. To be supported on @ModelAttribute methods in @Controller or @ControllerAdvice classes. Basically an @MVC-style handler method interception.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 14, 2014

Manuel Jordan commented

Sounds great!, thank you very much!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 7, 2016

Rossen Stoyanchev commented

Actually method references don't really apply for this purpose, i.e. are not usable outside of lambda expressions.

@spring-projects-issues spring-projects-issues added status: declined type: enhancement in: web labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web status: declined type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants