Mark Needham

Thoughts on Software Development

Archive for the ‘asp.net’ tag

ASP.NET MVC: Reducing duplication for partial models

with 3 comments

One of the problems we can encounter when using partials throughout our views is how we should create the model needed for those partials.

The approach that we have been following is to have the partial/child model on the parent model and then just call the appropriate method where we create the partial.

e.g.

public class ParentModel 
{
	public string Property1 {get;set;}
	public ChildModel ChildModel { get;set; }
}
 
public class ChildModel
{
	public string Property1 {get;set;}
}

We have sometimes run into the problem where the data in the ChildModel is being populated from the ParentModel (due to it also being needed there) leading to data duplication.

1
2
3
4
5
6
ParentModel parentModel = new ParentModel();
parentModel.Property1 = "value1"
parentModel.ChildModel = new ChildModel 
						{	
							Property1 = parentModel.Property1;
					  	}

Now the other problem with this is that we are relying on line 2 being executed before line 3 – we have created an order dependency in our code for no gain!

We are following a convention of having minimal logic in our views which means that we want to avoid creating the ChildModel in our view, meaning that we now have a problem to solve.

A cool approach which Dave introduced me to makes use of the Adaptor pattern to solve the problem.

We would adjust the ParentModel like so:

public class ParentModel
{
	public IChildModel { get { return new ChildModelAdaptor(this); }}
}

We then just delegate the calls to the ParentModel and drive the ChildModel to become an interface since it no longer needs to be a class.

public interface ChildModel
{
	string Property1 {get;set;}
}
public class ChildModelAdaptor : IChildModel 
{
	private ParentModel parentModel;
 
	public ChildModelAdaptor(ParentModel parentModel)
	{
		this.parentModel = parentModel;
	}
 
	public string Property1
	{		
		get { return parentModel.Property1; }
	}
}

If the data on the ChildModel is completely independent of the ParentModel then I would probably just create the model like before.

If the data on the ChildModel is a combination of data from the ParentModel and other classes then I would pass in those other classes in the constructor of the adaptor.

Written by Mark Needham

March 3rd, 2009 at 11:55 pm

Posted in .NET

Tagged with ,

ASP.NET MVC: Preventing XSS attacks

with 12 comments

XSS(Cross site scripting) attacks on websites seem to be quite popular these days but luckily if you’re working with the ASP.NET MVC framework Steve Sanderson has written a great post on how to protect yourself from this.

The solution Steve details works the opposite way to other solutions I have heard for this problem – we assume that everything that goes to the browser needs to be HTML encoded unless otherwise stated.

The benefit of this approach is that when we add new things to our page in the future we don’t need to worry that we might have inadvertently reintroduced a security problem.

Excluding aspx code from being encoded

After setting up the encoding we came up against the problem of how to exclude HTML that we are generating from our aspx templates that we don’t want HTML encoded.

This was a particular problem with our use of the HtmlHelper and UrlHelper provided with the framework.

Since the majority of the methods we use are extension methods on these classes we didn’t find it that easy to work out how to proxy those calls (we couldn’t just override the methods since they aren’t actually on HtmlHelper!) to our own RawHtmlHelper which could cast the result to RawHtml and therefore allow it to be rendered by the page.

The aim was to minimise the amount of things that we had to change. The worst case solution would be to have to go through all our aspx files and change every call to a Helper method to a new encoded version of the same method.

Luckily the new keyword comes to the rescue and allows us to hack around the problem.

We hooked into the page lifecycle to create a new version of the Html property which returned our RawHtmlHelper.

public class BaseViewPage<TModel> : ViewPage<TModel> where TModel = class
{
	public override void InitHelpers() 
	{
		base.Html = new RawHtmlHelper(..);
		base.Url = new RawUrlHelper(...);
	}
 
	public new RawHtmlHelper Html 
	{
		get { return (RawHtmlHelper) base.Html; }
	}
}
public class RawHtmlHelper : HtmlHelper 
{
	public RawHtml TextBox(string name, object value)
	{
		return (RawHtml) ((HtmlHelper)this).TextBox(name, value);
	}
}

In our aspx page we don’t need to make any changes to our code in order for it to now use the RawHtmlHelper method instead of the HtmlHelper method.

<%= Html.TextBox("name", "value") %>

The only other change was to make each of our aspx code behind classes inherit from BaseViewPage so that they receive the new Html property.

Overall

There is way too much casting for my liking in this solution but I’m not sure if a better solution exists other than creating our own version of the helper methods named ‘RawTextBox’, ‘RawRadioButton’ and so on.

The amount of work with that approach would be almost the same except if we have used the original HtmlHelper anywhere then we need to go and change all those too – exactly the problem we were trying to solve with the above solution.

Written by Mark Needham

February 12th, 2009 at 10:47 pm

Posted in .NET

Tagged with ,

Oxite: Some Thoughts

with 4 comments

The recently released Oxite code base has taken a bit of a hammering in the blogosphere for a variety of reasons – the general feeling being that it doesn’t really serve as a particularly good example of an ASP.NET MVC application.

I was intrigued to read the code though – you can always learn something by doing so and reading code is one of the ares that I want to improve in.

So in a style similar to that of a Technical Retrospective these are my thoughts.

Things I like

  • The Repository pattern from Domain Driven Design is used for providing access to the entities and provides encapsulation around the LINQ to SQL code.
  • There is some quite clever use of extension methods on FormCollection in the AdminController to create Posts and Tags from the data collected from the page. There are others as well – the developers really seem to grok extension methods in a way that I currently don’t. It will be interesting to see if I start using them more as I code more with C# 3.0.

Things I’d change

  • Testing wise there are some tests covering the MetaWeblogService class although it feels like the setup in each of the tests is a bit heavy. I’d refactor the tests using compose method to try and remove some of the noise and rename the tests so that they follow the BDD style more closely.
  • When time is used in the code it is done by directly using the DateTime class. Testing time based code is made much easier when we can control the time in our tests, so have a Time Provider or System Clock class can be a very useful approach here.
  • A classicist approach has been taken towards testing with no mocks in sight! It feels like a lot of work went into creating the fake versions of the classes which could maybe have been done much more easily using a stub in Rhino Mocks for example.
  • A lot of the ViewData code is weakly typed by putting values into an array. We started with this approach on a project I worked on and it’s so easy to misspell something somewhere that we came to the conclusion that if we can get strong typing we should strive for that. Jeremy Miller’s Thunderdome Principle works well here.
  • AdminController seems to have a massive number of responsibilities which is perhaps not surprising given the name. I think it’d be better to have the administrative tasks handled from the individual controllers for those tasks. This would also help lead the design towards a more RESTful approach
  • The Domain model is very driven from the definition of the database tables from my understanding. The domain objects are effectively representations of database tables. If we want to create a richer domain model then it would make more sense to design the classes in terms of what makes sense in the domain rather than what makes sense at a database level. An ORM tool could then be used to map the database tables to the code.
  • A lot of the domain objects implement an interface as well which I don’t think is really necessary. Although there are exceptions, in general when we refer to domain objects we care about the implementation rather than a contract describing what it does.
  • The Routes collection is passed around quite a few of the controllers, seemingly so that URLs can be generated inside other pages. It seems a bit overkill to pass this around to achieve such a simple goal and my thinking is that maybe a wrapper class which generated the URLs might be more intention revealing.
  • There is a bit more logic than I’m comfortable with in some of the views – I think it would be good to move this logic into ViewModel classes. This will have the added benefit of allowing that logic to be tested more easily.

Want to know more about

  • I’m curious as to why LINQ to SQL is being used as the interface to the database as I was under the impression that it is being phased out by Microsoft. The syntax seemed quite readable but I think the problem of interacting between code and the database in a clean way has been largely solved by NHibernate although the Entity Framework is a newish addition in this area.
  • One interesting thing I noticed was a lot of Background Services running in this codebase – I’ve not come across this in a web application before. They are actually being used for creating trackbacks, sending trackbacks and sending email messages.

My learning from reading the code

  • I asked for some advice on the best way to read code on Twitter and the most popular advice was to debug through the code. Unfortunately I couldn’t seem to do this without having a database in place so another approach was necessary. Instead I started reading from the tests that were available and then clicked through to areas of interest from there. I think it worked reasonably well but it wasn’t as focused as if I had debugged and I couldn’t see the state of the program as it executed.
  • I wanted to find a way to read the Oxite code and navigate to areas of the ASP.NET MVC source using Reflector without having to do so manually. TestDriven.NET was recommended to me and it worked really well. Clicking the ‘Go to Reflector’ option from the menu takes you to the current class in the Reflector window. Impressive.
  • Changing the Resharper find usages menu to show ‘Namespace + Type’ makes it much easier to try and work out what the code is doing rather than the default setting of just ‘Namespace’.
  • From looking at some of the ASP.NET MVC code I realised that a lot of data is stored in static variables in order to make the data globally accessible. It’s something I had never considered this before and it makes sense in a way but feels a little nasty
  • I found that I was getting side tracked quite a lot by irrelevant details in the code. I’m used to having a pair guide me through a new code base so looking at this one alone was a bit different for me. Separating noise/signal when reading code and identifying common patterns to allow me to do this is something I am working on.

Overall

I think it’s really cool that the Oxite team put their code out there for people to look at and learn from. A number of highly experienced developers have made suggestions for improvement so clearly this is quite a useful way to get feedback and code better in the future.

From what I understand, Rob Conery is working on some refactorings for this code base so it will be interesting to see what it looks like when this is done.

Written by Mark Needham

December 31st, 2008 at 1:26 am

Posted in .NET,Reading Code

Tagged with , , ,

Html.RadioButton setting all values to selected value workaround

with one comment

While working with the Html.RadioButton() UI helper for ASP.NET MVC we came across an interesting problem whereby when you submitted the form, all the values for that particular group of radio buttons was set to the value of the one that was selected.

For example, given a form like this:

1
2
<%= Html.RadioButton("option1", true) %>Yes
<%= Html.RadioButton("option2", false)%>No

When we first load the page, this is the HTML it generated:

1
2
<input type="radio" name="option1" value="true" />Yes
<input type="radio" name="option1" value="false" />No

When we post the form having selected the ‘Yes’ option for example, this is what the HTML looks like now:

1
2
<input type="radio" name="option1" value="true" checked="checked" />Yes
<input type="radio" name="option1" value="true" />No

A bit of Googling revealed that others have come across this same problem and that it is a bug in the code.

The solution suggested on Stack Overflow was to write a custom RadioButton helper which does a regex replacement on the ‘value=’ part of the HTMl generated.

We started working down the path to using a similar approach before James pointed out that we might be able to achieve the same outcome by passing in the value as one of the htmlAttributes.

We changed our original Html.RadioButton() code to take in ‘new { value = true }’ and ‘new { value = false }’ respectively and it solved the problem.

Written by Mark Needham

November 28th, 2008 at 9:32 pm

Posted in .NET

Tagged with ,

Debugging ASP.NET MVC source code

with one comment

We’ve been doing some work with the ASP.NET MVC framework this week and one of the things we wanted to be able to do is to debug through the source code to see how it works.

Our initial idea was to bin deploy the ASP.NET MVC assemblies with the corresponding pdbs. Unfortunately this didn’t work and we got a conflict with the assemblies deployed in the GAC:

Compiler Error Message: CS0433: The type 'System.Web.Mvc.FormMethod' exists in both 'c:\WINDOWS\Microsoft.NET\Framework\v2.0.50727\Temporary ASP.NET Files\root\8553427a\c1d1b9c6\assembly\dl3\898a195a\60680eb9_3349c901\System.Web.Mvc.DLL' and 'c:\WINDOWS\assembly\GAC_MSIL\System.Web.Mvc\1.0.0.0__31bf3856ad364e35\System.Web.Mvc.dll'

We attempted to uninstall the System.Web.Mvc assembly from the GAC but were unable to do so because it has other dependencies.

The next idea was to uninstall ASP.NET MVC using the MSI uninstaller. This worked in terms of getting rid of the assembly from the GAC but it meant that we no longer had support for ASP.NET MVC in Visual Studio.

Luckily Troy pointed out James Kovac’s blog post about debugging the .NET framework source and we were able to debug the ASP.NET MVC code by hooking up the pdbs that come with the source code download.

Some other approaches were pointed out on the ALT.NET mailing list although the above is what worked for us.

Written by Mark Needham

November 19th, 2008 at 9:30 pm

Posted in .NET

Tagged with ,