aspose file tools*
The moose likes Servlets and the fly likes Design question re: Passing HttpServletRequest is a no-no Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Servlets
Bookmark "Design question re: Passing HttpServletRequest is a no-no" Watch "Design question re: Passing HttpServletRequest is a no-no" New topic
Author

Design question re: Passing HttpServletRequest is a no-no

Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 318
    
    2

Hi All,

I'm in the process of cleaning up and extending a Web Application written a couple of years ago by a complete Java EE noob (me).

One of my mistakes was to pass the request and response object around too much. In most cases it is clear how to refactor as the class only needs specific information, but I have a couple of cases where encapsulation desires suggest that the servlet should not need to know what data the class needs.

My current conundrum involves a couple of database tables that are used to generate usage statistics. At the start of a session (request.getSession().isNew()) we record: who the user is, the user-agent, remote-ip, keep track of how many visits, and first and last visit. This of course changes to answer questions that come up when I request bigger machines to run the server.

The current plan is a Data Access Object that can take a request object or a ResultSet entry in the constructor. There is also a Table class to manage the db.

Once again, the exercise of putting the problem in words seems to have reinforced my position, but I am very interested in any suggestions or comments as I'm still learning.

Thanks,
Joe


It's not what your program can do, it's what your users do with the program.
Bear Bibeault
Author and ninkuma
Marshal

Joined: Jan 10, 2002
Posts: 61224
    
  66

I firmly believe that passing either of a servlet artifact or a result set to another class is a big mistake. Extract the data from the request, or the result set, and pass that.


[Asking smart questions] [Bear's FrontMan] [About Bear] [Books by Bear]
J. Kevin Robbins
Bartender

Joined: Dec 16, 2010
Posts: 956
    
  13

I agree with Bear. Servlets and filters understand HTTP objects. Nothing else should know or care. ResultSets should never leave the DAO. Use Maps, Lists, or Objects to pass information around to other classes.


"The good news about computers is that they do what you tell them to do. The bad news is that they do what you tell them to do." -- Ted Nelson
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 318
    
    2

Bear Bibeault wrote:I firmly believe that passing either of a servlet artifact or a result set to another class is a big mistake. Extract the data from the request, or the result set, and pass that.


Bear,

I value your opinion highly and really want to discuss this further to find the problems with my understanding of the problem.

I disagree with your statement, probably out of ignorance, so please bear (no pun intended) with me.

I believe the fundamental concept here is encapsulation IOW the need to know.

The general case is a class has an object filled with artifacts and another class needs some of those, exactly which ones will probably change over time. The subset if defined in the first class as well as the second means a change in the contents of the subset needs to modify both. Whereas if we pass the object to the second class it and only it needs to know what it needs.

I do understand that HttpSevletRequest and ResultSet are ..what's the word.. very implementation dependent (maybe) but the class does have a default constructor and getter/setter methods.

Again I value your opinion, what's wrong with mine?

Thanks,
Joe
Bear Bibeault
Author and ninkuma
Marshal

Joined: Jan 10, 2002
Posts: 61224
    
  66

There are a number of ways to look at the issue: an important one is coupling. By passing a servlet request to a class that doesn't need anything form the request except some data that it holds (perhaps in the parameters), you have strongly coupled the class to the servlet environment and rendered it useless for any other purpose. This is bad not only for re-use, but adds complexity and violates the principle of modularity.

You might argue that "I don't plan to use the class anywhere else". But that's a novice attitude. You don't know what the future holds. And even if it turns out to be true, the extra complexity, which could have ramifications at both build and execution times, are real.

Separation of Concerns is an important principle in programming. If a class doesn't need to know about servlets, and you are just passing a servlet request to it for convenience, you have violated the principle. If a class doesn't need to know about databases, passing a result set to it for convenience violates the principle.

Seasoned developers know that just getting something to work is not enough. Principles such as modularity, loose coupling, and Separation of Concerns exist and are bandied about because they have proven time and time again that they make code more robust, less fragile, and easier to work with as projects progress.

I know. I've been around long enough, and have done it wrong many times, and have the scars to prove it.
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 318
    
    2

J. Kevin Robbins wrote:I agree with Bear. Servlets and filters understand HTTP objects. Nothing else should know or care. ResultSets should never leave the DAO. Use Maps, Lists, or Objects to pass information around to other classes.

Kevin,
This is a DAO. But it is trying to encapsulate both an HTTP object and a row in a Table.

Thus my confusion.

Joe
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 318
    
    2

Maybe I should expound a bit.

My collaboration uses Shibboleth. So what we need from the request object is contained in attributes, headers and specific calls. Creating a Map object ... a hassle but doable. Maybe that's what I'm missing.

Joe
J. Kevin Robbins
Bartender

Joined: Dec 16, 2010
Posts: 956
    
  13

Joe Areeda wrote:
J. Kevin Robbins wrote:I agree with Bear. Servlets and filters understand HTTP objects. Nothing else should know or care. ResultSets should never leave the DAO. Use Maps, Lists, or Objects to pass information around to other classes.

Kevin,
This is a DAO. But it is trying to encapsulate both an HTTP object and a row in a Table.

Thus my confusion.

Joe

I have to admit that statement confuses me a bit. A DAO should accept the parameters (strings, ints, whatever) needed to complete the SQL statement which is defined as a PreparedStatement or CallableStatement (stored procedure). It will take the ResultSet and load the results in a Map or List or bean, whatever is more appropriate, immediately close the ResultSet, Statement, and Connection, and return the results to the caller. The caller might be a servlet in a web app today, or a POJO in a desktop app tomorrow, or a mobile app next week, or something that hasn't even been thought of yet. See how the DAO stays reusable if you keep everything encapsulated?
Jayesh A Lalwani
Bartender

Joined: Jan 17, 2008
Posts: 2377
    
  28

Your terminology is confusing me a little bit. A DAO is encapsulating an HTTPRequest/data row. That makes no sense to me. I can understand a Data Transfer Object or a Value Object encapsulating a HTTPRequest/data row. I can understand a DAO using a HTTPRequest/data row. the DAO is not suppossed to be a layer around the real persistence storage that holds state. It's not suppossed to hold state itself. So, why is it encapsulating things that hold state?

Reading between the lines, it sounds to me like you are conflating Data Transfer Object with Data Access Object. SO, going forward with the assumption that you want to encapsulate Servlet Request inside a DTO... or IOW your DTO is a bridge object between your DAO and the HTTPRequest. It's hiding the HTTP Request from the DAO layer while providing a easy way to get the relevant data from the request.

To me, this is not a terrible idea, but I'm more of a understand-the-principles-rather-than-stick-to0-them-blindly kind of guy. If you want to pass Java certification what I think doesn't matter.

The thing is it breaks certain design principles. You might be able to get around these problems by changing your design

1) Separation of concerns
Like Bear was saying, your DAO layer shouldn;t be dependent on servlet API. Yes, I understand you are hiding the servlet API from your DAO object by encapsulating the HTTPServletRequest inside your DTO. However, you are adding a build dependency. If your DTO needs the servlet APi to be on classpath, that means you cannot use that DTO out of a container. If your DAO is tied to the DTO, then your DAO cannot execute outside of a container. That's a bad thing. Tommorrow, you want to build a command line tool that loads data into your database, you won;t be able to use it because the servlet API is not available outside of the container

Yes, you can get around this by declaring your DTO as an interface rather than a concrete object. The interface can be declared in the DAO module, and the Application layer can implement the interface, and it's implementation can wrap HTTPServletRequest. THis way if you are building a command line tool, the command line tool can have it's own implementation that implements it as a POJO. I would say this design is a little clunky compared to the reccomended alternative:- implement DTO as a POJO, and just convert Request to the POJO

There are advantages too. It prevents the big blocks of getter/setter calls that you usually make when you want to extract data in and out of an POJO. The code will definetly be cleaner. Most modern implementations that rely on DTO being a POJO address this by converting JSON/XML directly to the object. The code is clean and you have seperation of concerns

2) Dependency to environment
There is an assumption that after a DTO carries state and only state, and no other environmental changes will alter the state. It's a rather pure object. You could hold on to that object forever and ever and it will never change. You make this assumption in your code everywhere, because it;s one of the founding principles of MVC. Model holds state. State never changes magically. In your case, since the state encapsulates something that is bound to the Http request, your state is valid only in the context of that request. You can't hold on to that DTO object anymore
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 318
    
    2

Thank you all for the discussion.

I have to digest all of this.

Joe
William Brogden
Author and all-around good cowpoke
Rancher

Joined: Mar 22, 2000
Posts: 12788
    
    5
Number one reason for not passing request or response object references: You can't test your code outside the servlet environment!!

Figure out how to use Maps, etc to contain the data and streams to handle output - then your code / test / modify cycle will be faster and it will be easier to measure performance.

Bill
Joe Areeda
Ranch Hand

Joined: Apr 15, 2011
Posts: 318
    
    2

I'm convinced. I'm working on how to solve it.

One of the issues that has lead me down this path is the way Shibboleth passes its artifacts through Apache to Tomcat. They can be passed as attributes or header fields. Using headers is frown upon because it can be spoofed by a malicious client. However something is weird about passing them as attributes. While getAttribute works, getAttributeNames does not return them, it's been a while but I believe it's an issue with Tomcats AJP connector. Evidently Glassfish does not have this problem but the Shibboleth set up is very complicated.

So my conceptual problem is that a data object that has what we need from the request object would look much like the request object. Parameters and headers are easy, attributes get pulled one by one plus things like remoteip, contentType, contentLength have getters. The other things we use from the request object have already been moved to the servlets or their helper classes.

I'm glad I asked the question. It's kept me from doing something I have to refactor out later, and that list is already long enough.

Best,
Joe
 
Consider Paul's rocket mass heater.
 
subject: Design question re: Passing HttpServletRequest is a no-no