0

I am working with Spring MVC controller. I have one of my controller as DataController.

I am thinking to add HttpServletRequest as injectable at the top of DataController class using @Inject.

@Controller
public class DataController {

    @Inject
    HttpServletRequest request;

    // .. some code here

    @RequestMapping(value = "process", method = RequestMethod.GET)
    public @ResponseBody
    DataResponse processTask(@RequestParam("workflow") final String workflow) {

        String ipAddress = request.getRemoteAddr();
        System.out.println(ipAddress);

}

So my question is - Is this the right way to use @Inject? I have never used @Inject before so trying to learn whether the way I am doing it is right or not? Since everytime, who is making call to processTask method, I need to grab its ipAddress whoever is calling that processTask method.

Sotirios Delimanolis
  • 274,122
  • 60
  • 696
  • 724
john
  • 11,311
  • 40
  • 131
  • 251
  • 1
    You could just try it... – Sotirios Delimanolis Jun 10 '14 at 23:22
  • Does that controller need a single `HttpServletRequest` over its lifetime? Remember that a controller is, at heart, just another object. The request should normally be passed in as a method parameter, since the method gets called once for each request. – chrylis -cautiouslyoptimistic- Jun 10 '14 at 23:26
  • @chrylis That will be a request scoped bean, different value for each request. – Sotirios Delimanolis Jun 10 '14 at 23:28
  • @SotiriosDelimanolis What do you mean by "that"? The code provided is a default singleton-scoped. (And method injection is the Right Thing as far as testing and modularity anyhow.) – chrylis -cautiouslyoptimistic- Jun 10 '14 at 23:31
  • @chrylis Spring will inject a proxy of `HttpServletRequest` which will contain an `ObjectFactory` which will retrieve the _actual_ `HttpServletRequest` when requested (ex: method invocation). This can only work in a context where that request is available (like here). It will fail anywhere else. – Sotirios Delimanolis Jun 10 '14 at 23:33
  • @SotiriosDelimanolis I wasn't aware that Spring would auto-scope the request, just for special-purpose ones like `PersistenceContext`. Interesting. (Still support method injection; I'm a recent convert, especially when adding Groovy constructor syntax!) – chrylis -cautiouslyoptimistic- Jun 10 '14 at 23:35
  • @chrylis Don't get me wrong, method injection is the proper use with Controllers. But that's only because Spring uses reflection to invoke handler methods. This (method injection) doesn't work for any other bean that needs the `HttpServletRequest`. – Sotirios Delimanolis Jun 10 '14 at 23:36
  • @SotiriosDelimanolis It works fine but my doubt is suppose I am making two different requests from two different computers to `processTask` method then ipAddress will be different from both the calls right in `request` object, if I declared `HttpServletRequest` like that? – john Jun 10 '14 at 23:43
  • You will be fine for the reasons expressed above. However, as @chrylis stated, you should ideally make the `HttpServletRequest` a method parameter in your `@Controller` `@RequestMapping` annotated methods. – Sotirios Delimanolis Jun 10 '14 at 23:44
  • Sure but in that case, I cannot call `processTask` method with one parameter, right? Or is there any way to make it optional? – john Jun 10 '14 at 23:49
  • You aren't the one calling `processTask`, Spring is. See the Spring documentation section on supported method arguments. – Sotirios Delimanolis Jun 11 '14 at 01:24

2 Answers2

0

In terms of acquiring HttpServletRequest: semantically speaking, it is definitely wrong.

Reason: HttpServletRequest is an object that is created only when users send requests and is destroyed once the requested user action is completed. You simply can store it that way (from syntax angle) but you shouldn't (from semantic angle). You need to realize that the way how web application works is not exactly same as a desktop application (and don't observe them from the same angle).

Suggestion:

@RequestMapping(value = "process", method = RequestMethod.GET)
public @ResponseBody
DataResponse processTask(@RequestParam("workflow") final String workflow, HttpServletRequest request) {...}

In this way you will get the corresponding request each time the processTask method is called. (HttpServletRequest object is injected by @RequestMapping.)

(If you would like to preserve something through out a session, consider use a bean that is

Suggestion:
@Inject private UserService userService;
(assume we have a class registered called UserService.)

conan_z
  • 460
  • 6
  • 17
  • Both statements `You simply can't store it that way and you shouldn't.` are wrong. You can store it that way. The only reason it wouldn't make sense to store it there is if only some of the request handlers used it. If they all need it, you should use an instance field like OP has shown. – Sotirios Delimanolis Jun 11 '14 at 01:21
  • So is: `If you are using Spring, then you really should be using @Autowired rather than @Inject`. Either one is fine. Since `@Inject` comes from a standard Java EE component, you can more easily make your application modules not depend on Spring and be re-usable with other IoC containers. – Sotirios Delimanolis Jun 11 '14 at 01:21
  • @SotiriosDelimanolis Thanks for the correction, I agree with your opinion. But for that _You simply can't store it that way and you shouldn't._ statement, that is from semantic angle. Semantically I still don't think he should be storing that way. – conan_z Jun 11 '14 at 14:04
-1

You cannot "inject" the HttpServletRequest the only way to use it as far as I know is to added as a method member. Like this:

@Controller
public class DataController {



    // .. some code here

    @RequestMapping(value = "process", method = RequestMethod.GET)
    public @ResponseBody
    DataResponse processTask(@RequestParam("workflow") final String workflow,HttpServletRequest request) {

        String ipAddress = request.getRemoteAddr();
        System.out.println(ipAddress);

}

look also at Spring MVC @AutoWired response not working

Community
  • 1
  • 1
Cross2004
  • 931
  • 1
  • 7
  • 12