Posts tagged with 'PostSharp'

Welcome to another "Weekly Concerns". This is a post-a-week series of interesting links, relevant to programming and programmers. You can check out previous Weekly Concerns posts in the archive.

If you have an interesting link that you'd like to see in Weekly Concerns, leave a comment or contact me.

It's the last part of the series, and I just thought I would wrap things up by talking about unit testing the aspect. I wrote a whole chapter in my book AOP in .NET on unit-testing of aspects. I also was honored to do a live webinar with Gael Fraiteur on unit testing with PostSharp, in which he has a different view on aspect unit testing, so I recommend you check that out too.

I'm not straying too much from the book in this example, so if you're familiar with that chapter, there's not much new for you here.

My plan is to not test the PostSharp aspect directly, but instead have the aspect delegate just about all the functionality to a plain old C# class that's easier to test. The old adage of solving a problem by adding another layer of indirection applies. Doing it this way makes my aspect easier to test, further decouples my application from a specific framework, but does require some more work, so it's not like it's a magic cost-free approach to testing either.

Here is the aspect, all tightly coupled to PostSharp:

And here is my refactored code, which you'll notice results in a higher number of classes, but it is ready to unit test. You may also notice that some of the logic is a bit different than the above aspect. This is because I found some issues and bugs while I was writing unit tests. So, hooray for unit tests! Also notice the "On" static field. I touch on this in the book, but this flag is merely there so that I can unit test the actual services while isolating them from the aspect. Since PostSharp uses a compile-time weaver, this is necessary. If you are using a runtime AOP weaver like Castle DynamicProxy, that's not necessary.

I'll even show you some of my unit tests, which I've written in mspec and JustMock Lite with the help of Sticking Place for testing the SqlException stuff.

Notice that I moved the Exceptionless-specific code behind an interface (IExceptionLogger) as well, to further decouple, improve testability, and also because Exceptionless is just the sort of thing that might actually be swapped out somewhere down the line. I've left out the details of that implementation because it's quite trivial.

So now you are caught up to today: this is the aspect I'm actually using and deploying to a staging site for my cofounder and our designer to use and test. And so far I must say that it's working quite well, but things could always change when it actually hits real customers, of course.

I'm starting to wind down this blog post series. A couple more tweaks and you'll be all caught up with how I'm using this aspect in a system that I'm actively working on right now.

But first, let me address some of the criticism that I've received in comments, reddit, and so on.

Mainly, the criticism about my aspect making the return value be an empty collection (instead of null) if the return type is a collection. I totally expected this criticism, and I definitely think it's fair. I pondered this idea a lot myself before I came to the conclusion that I did, and it's entirely possible that I'll change my mind later. So, if you are following along, and thinking about creating an aspect like this, you should definitely think carefully about doing this; don't just copy and paste my aspect and call it a day.

I think it comes down to a choice: do I want to always be checking for nulls, or when there's a collection can I just assume that it will (at worst) be empty? I based my decision somewhat on the Framework Design Guidelines - my API should return an empty collection (or array) instead of nulls. My thought process was also that this aspect is responsible for the return value being null in the first place (because it's handling the exception and not bubbling it up), and therefore it should make a reasonable attempt at not disrupting normal program flow by returning an empty collection.

This is an opinionated approach, and it may not mesh with what you are trying to do, you or your team's coding style, and where you intend to use this aspect.

Okay, on with the show.

The aspect I've written is able to send an error message up through the stack and eventually to the UI (in my application right now, that's either MVC ModelState or a display element in my Telerik Reporting reports, but it could easily be other UI elements too). Here's the relevant bit of code:

While I'm developing, that's great. I get the exception message and it helps me pinpoint the error. There's no stack trace, but you could add that too if you wanted (or check it out on Exceptionless logs). But there's a problem: I do not want my users (especially any malicious ones) to see detailed error messages like that. So, just add in some configuration and logic to show (or not show) a verbose error message. It's a piece of cake, and it really isn't something that's specific to AOP (other than to show that AOP code can be configured just like any other part of your code). I also want to refactor this to only use Exceptionless when in production: I don't need to log any exceptions that I create locally when I'm in the process of writing code.

Notice that I kinda hand-waved the configuration stuff away into the staic class/method IsThisSite.RunningLocally? The RunningLocally code is simply a check of Web.config (or App.config) using ConfigurationManager.AppSettings. How you manage your configurations is up to you and your team.

Also notice that GetErrorMessage could be customized to show different error messages depending on the exception (or not).

Okay, so I think this aspect is in pretty good shape. It's helpful, cuts down on boilerplate, and implements a very important cross-cutting concern requirement (error handling/logging). But, a) you shouldn't have to take my word for it that it works, and b) what if I want to refactor it? It should be unit tested (which is the next and final post).

In the last post in the series, I moved the error handling aspect into an opinionated one by using Exceptionless and handling certain types of exceptions (like SqlException and NotImplementedException).

In this post, I'm going to go even further down the opinionated road and talk about exception recovery.

My goal is for any exception to be caught and logged, and we're already there. In addition, I'd like the user not to see a yellow screen, so I'm generally not allowing the exception to bubble up, in lieu of a nice error message being put into ModelState (or some other error handling). However, this might not be enough. If the service method throwing an exception has a return value, then it's likely that another error will happen later in execution (maybe due to a null reference). For example:

If the method doesn't throw an exception, everything is fine. If it does throw an exception, there will a null reference when I attempt to use the Count property. What I would prefer is that an empty collection is returned. In fact, this fits with the recommendations from Framework Design Guidelines (page 256) to always return an empty collection (or array) instead of null. I've added a helper method to construct the return value in case this occurs.

What about if the service method is not returning a collection? And what if it's returning a value (like int or DateTime)? It's okay to return a null in the first case. In the second case, it might be okay to throw an exception. I have so few of these in the project I'm working on that I don't worry about it much, but for the sake of completeness, let's just make it return the default value.

I need to be very cautious about what I'm doing, and wrap everything in a try/catch so that I don't end up causing another unhandled exception in my exception handling code. It would be a good idea to create some unit tests to make sure everything is behaving as expected.

Okay, I think one or two more posts will wrap it up for this series. We need to discuss whether or not the exception message should be shown to a user (hopefully you're leaning towards "no").

  • This only works for ITerritoryService
  • Swallowing exceptions: we should at least be logging the exception, not just ignoring it.
  • If the method has a return type, then it's returning null. If it's meant to return a reference type, that's probably fine, but what if it's returning a value type?
  • What if it's returning a collection, and the UI assumes that it will get an empty collection instead of a null reference?
  • Do we really want to return the exception message to the user?
  • What if I forget to use ServiceBase as a base class on a new service?
  • Unit tests: how to test this aspect in isolation.

 

The Boise .NET User Group was kind enough to invite me to be a presenter. Boise is a bit too far away for me to make it there on a weekday on a small budget, but they were accomodating enough to allow me to present remotely. There were a couple minor technical issues, but mostly it went well. Even though I've done this presentation a lot, Brian cajoled me into talking about INotifyPropertyChanged, which is something I've only done once before during this talk (usually I'm constrained for time, or the group isn't interested in the MVVM pattern).

Brian Lagunas even made a recording of the presentation. The recording started a minute or so late, and it ending a couple minutes too early (the internet in the Boise location went down), but most of it is was recorded with Microsoft Live Meeting (not my favorite remote collaboration tool, but it was adequate). And now, it's on YouTube:

Slides and code are available in the AOP For You and Me GitHub repository.

Matthew D. Groves

About the Author

Matthew D. Groves lives in Central Ohio. He works remotely, loves to code, and is a Microsoft MVP.

Latest Comments

Twitter