views:

366

answers:

5

I'm building an application that roughly follows the repository pattern with a service layer on top, similar to early versions of Conery's MVC Storefront.

I need to implement a page that returns all users except for the current user. I already have GetUsers() methods on the repository and service layers, so the question is where to apply the "except for the current user."

Should the service layer be aware of HttpContext, thus applying this rule? I am tempted to just pass in the current user (id) from the controller to this service method, but it seems cleaner if the service layer was HttpContext-aware and could do this on its own.

One obvious alternative is to apply this rule directly within the Controller, but I'm just not hot on that idea...


Edit - just to comment on the answers: I see the issues with the reverse dependency problem, something I was completely overlooking. I'm marking Mehrdad's as the answer due votes, but everyone has really provided a valuable response worth reading!

+9  A: 

Absolutely not. My mindset in designing these kind of things is like this: I assume I need to write a Windows based app along with the Web application and try to minimize dependency on Web specific stuff. Passing HttpContext directly will increase coupling of your service layer to your Web UI layer which is not ideal.

Mehrdad Afshari
+3  A: 

You should not create a reverse dependency between your service layer and the web tier. Consider what happens when you want to extend your service layer to work with a forms-based application or windows service. Now you've got to work around the web dependency to get your same methods to work or duplicate some (perhaps, small, but still duplicate) code. You would be better served to extract the user's identifier into something useful in the context of the service layer and use that value with the service layer. Handling the filtering on the web site is also acceptable, though if you do it more than once it would need to be refactored into a common place and the service layer is the natural spot for it.

tvanfosson
+1  A: 

I find it good practice to build a custom AppContext class which contains my current user object (as well as other contextual data). This class has no references to System.Web. Any service methods that need to be context aware should have an AppContext parameter (e.g. for checking security rights, or getting the current user as in your case). Populate this object in the web-tier and keep it in session if you need to. This way your service layer knows nothing about System.Web.

JonoW
What's wrong with simply passing user Id?
Mehrdad Afshari
Nothing wrong with it at all, I just like treating all context-aware situations in the same way.
JonoW
I like this idea, JonoW! I may just pass the id for now, if/when my service methods start getting ugly with params and overloads this would be great. I could even pass the AppContext when instantiating the service object, thereby fulfilling my desire to have the service somewhat "aware" but not dependent on System.Web.
Kurt Schindler
Thanks :) I guess it does depend on how many of your service methods need to be context-aware. In our current project, all of them are, as every service method has different security restrictions (defined by attributes), so every service method call needs to look at the Roles of the current user to check if they are allowed to call this method. Could be way overkill though!
JonoW
If the number of parameters is an issue, just encapsulate them in some class/struct. No need for making them context-oriented.
Mehrdad Afshari
+5  A: 

The answer is, no.

Not only should the Service Layer have no dependency on the current Presentation Layer, in my opinion it should have no dependency on the current application. For instance, I would not use a custom AppContext class as JonoW suggested here.

Instead, pass the current user as a parameter to the GetAllUsersExceptForTheCurrentUser method.That way, the service can be used by any application that needs to process users, not only the current application.

John Saunders
Agreed. Specifically, you'll thank yourself for not adding *context* and *state* to a service layer the moment you need to put it behind a load balancer.
Mehrdad Afshari
Thanks, John, this is a great point. Makes my reconsider my intial reaction to JonoW's suggestion.
Kurt Schindler
I get what you guys are saying, but when I say a service method needs to be context-aware, I mean having a service method that needs to know who is calling it. For instance, how do you guys handle authorisation? i.e. determing if the method caller is allowed to call the method? I'm a bit of a newb in this arena, so keen to hear how others do it/what best practices are. Cheers
JonoW
"Single Responsibility Principal". A method should do its job, not be concerned about authorization. Leave that to the part that decides whether or not to call the method.
John Saunders
+1  A: 

No. Doing so will make your code harder to test and re-use.

I tend to build an infrastructure interface for this sort of thing (call it IAuthentication or something) and expose a CurrentUser property on it. Then I'd inject this into my service via its a constructor. i.e. public MyService(IAuthentication auth)

Finally you'd can build an HttpContext aware implementation of IAuthentication (say WebAuthentication).

Now when you create your service you create its dependencies as well:

var myService = new MyService(new WebAuthentication());
var otherUsers = myService.GetAllOtherUsers();

If you are using an IoC container the ugly dependency can even go away!

Wolfbyte