Sunday, 6 April 2014

Another Look at Unit Testing Umbraco Surface Controllers (Part 2)

In the previous post I looked at testing Umbraco surface controllers, using a technique that basically avoided the issue by extracting the non-Umbraco related logic for test into a separate class, and testing that.

Another option is to test the controller as is, using the base test classes provided by the Umbraco core team.

My starting point for working with these was a blog post by Jorge Lusar where he details much of the process for working with these. He used Umbraco 6.1.5, and I found things had changed a little in 7.1.0 (latest at time of writing), as well as needing some additional work to handle the controller methods I was looking to test.

As Jorge notes, the first things is to get the test classes. They aren't currently available as a separate download as far as I'm aware, so instead I've pulled down and built the latest source code, and then copied Umbraco.Tests.dll to my solution and referenced it from my test project.

I tend to use MSTest for my unit testing, but these base classes are based on using NUnit, so the first thing was to switch to that.

To recap, this was the controller method I was looking to test:

[HttpPost]
public ActionResult CreateComment(CommentViewModel model)
{
    if (!ModelState.IsValid)
    {
        return CurrentUmbracoPage();
    }

    TempData.Add("CustomMessage", "Thanks for your comment.");

    return RedirectToCurrentUmbracoPage();
}

And I have these two tests:

[Test]        
public void CreateComment_WithValidComment_RedirectsWithMessage()
{
    // Arrange
    var controller = GetController();
    var model = new CommentViewModel
    {
        Name = "Fred",
        Email = "fred@freddie.com",
        Comment = "Can I test this?",
    };

    // Act
    var result = controller.CreateComment(model);

    // Assert
    var redirectToUmbracoPageResult = result as RedirectToUmbracoPageResult;
    Assert.IsNotNull(redirectToUmbracoPageResult);
    Assert.AreEqual(1000, redirectToUmbracoPageResult.PublishedContent.Id);
    Assert.IsNotNull(controller.TempData["CustomMessage"]);
}

[Test]
public void CreateComment_WithInValidComment_RedisplaysForm()
{
    // Arrange
    var controller = GetController();
    var model = new CommentViewModel
    {
        Name = "Fred",
        Email = string.Empty,
        Comment = "Can I test this?",
    };
    controller.ModelState.AddModelError("Email", "Email is required.");

    // Act
    var result = controller.CreateComment(model);

    // Assert
    var umbracoPageResult = result as UmbracoPageResult;
    Assert.IsNotNull(umbracoPageResult);
    Assert.IsNull(controller.TempData["CustomMessage"]);
}

In order to get this working I needed to make one change to my controller, adding a second constructor that allows the passing in of an instance of an UmbracoContext, as follows:

public BlogPostSurfaceController(UmbracoContext ctx) : base(ctx)
{
}

One change from Jorge's method is that now rather than indicating to the base test classes that a database is not required by overriding a method - if it is, the base classes support set up of a SQL CE database instance - an attribute is added either to the test class or method. In my case I didn't need a database so could use this version of the attribute, and as you can see the test class itself inherits from BaseRoutingTest which is provided by the Umbraco base test classes.

[TestFixture]
[DatabaseTestBehavior(DatabaseBehavior.NoDatabasePerFixture)]
public class BlogPostSurfaceControllerTestsWithBaseClasses : BaseRoutingTest
{
    ...
}

In order to set up the necessary contexts for the test I've adapted Jorge's method GetController() which you can see called from the test methods above. The code for this is as follows, with further explanation below, as it got just a bit complex at this point!

private BlogPostSurfaceController GetController()
{
    // Create contexts via test base class methods
    var routingContext = GetRoutingContext("/test");
    var umbracoContext = routingContext.UmbracoContext;
    var contextBase = umbracoContext.HttpContext;

    // We need to add a value to the controller's RouteData, otherwise calls to CurrentPage
    // (or RedirectToCurrentUmbracoPage) will fail

    // Unfortunately some types and constructors necessary to do this are marked as internal

    // Create instance of RouteDefinition class using reflection
    // - note: have to use LoadFrom not LoadFile here to type can be cast (http://stackoverflow.com/questions/3032549/c-on-casting-to-the-same-class-that-came-from-another-assembly
    var assembly = Assembly.LoadFrom(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "umbraco.dll"));
    var reflectedRouteDefinitionType = assembly.GetType("Umbraco.Web.Mvc.RouteDefinition");
    var routeDefinition = Activator.CreateInstance(reflectedRouteDefinitionType);

    // Similarly create instance of PublishedContentRequest
    // - note: have to do this a little differently as in this case the class is public but the constructor is internal
    var reflectedPublishedContentRequestType = assembly.GetType("Umbraco.Web.Routing.PublishedContentRequest"); 
    var flags = BindingFlags.NonPublic | BindingFlags.Instance;
    var culture = CultureInfo.InvariantCulture;
    var publishedContentRequest = Activator.CreateInstance(reflectedPublishedContentRequestType, flags, null, new object[] { new Uri("/test", UriKind.Relative), routingContext }, culture);

    // Set properties on reflected types (not all of them, just the ones that are needed for the test to run)
    var publishedContentRequestPublishedContentProperty = reflectedPublishedContentRequestType.GetProperty("PublishedContent");
    publishedContentRequestPublishedContentProperty.SetValue(publishedContentRequest, MockIPublishedContent(), null);
    var publishedContentRequestProperty = reflectedRouteDefinitionType.GetProperty("PublishedContentRequest");
    publishedContentRequestProperty.SetValue(routeDefinition, publishedContentRequest, null);

    // Then add it to the route data tht will be passed to the controller context
    // - without it SurfaceController.CurrentPage will throw an exception of: "Cannot find the Umbraco route definition in the route values, the request must be made in the context of an Umbraco request"
    var routeData = new RouteData();
    routeData.DataTokens.Add("umbraco-route-def", routeDefinition);

    // Create the controller with the appropriate contexts
    var controller = new BlogPostSurfaceController(umbracoContext);
    controller.ControllerContext = new ControllerContext(contextBase, routeData, controller);
    controller.Url = new UrlHelper(new RequestContext(contextBase, new RouteData()), new RouteCollection());
    return controller;
}

private IPublishedContent MockIPublishedContent()
{
    var mock = new Mock<IPublishedContent>();
    mock.Setup(x => x.Id).Returns(1000);
    return mock.Object;
}

Having first taken Jorge's method, I ran into problems with the call to RedirectToCurrentUmbracoPage() in the controller method. This in turn makes a call to the CurrentPage property and that would fail without an appropriate value in the umbraco-route-def key of the controller's route data.

I've added that, but unfortunately the value for this key is an instance of a RouteDefinition class, provided by Umbraco but marked as internal. In order to create an instance of that, you have to resort to using reflection, which I've done above by referencing the assembly, activating an instance of the type, and setting the necessary properties.

Similarly, a property of RouteDefinition - PublishedContentRequest - whilst a public class, has an internal constructor. We can again use reflection though to create an instance of this, and set it's properties - in this case an instance of IPublishedContent that can be mocked using Moq.

The final step to getting this to work was to ensure the appropriate configuration files are in place - web.config and umbracoSettings.config. These I copied from the Umbraco.Tests project from source, added them to my project and set their Copy to Output Directory property to Copy Always, so they would be found in the location that the base test classes expect

Having done all this, the tests pass.

Conclusions

There is an important caveat with using this method which means we might question on using it in production. Classes, constructors and methods are marked as internal for a reason by the Umbraco core team, namely that by doing so they can safely change them without worrying about changing a public interface that might affect people's applications. You can get round this using reflection as I have, but it's at risk of breaking following any upgrade to Umbraco, even minor ones.

That said, this code is nicely isolated and only needs to be set up once for testing surface controllers, and so could likely be fairly easily updated should these internal classes change.

If any one wants to review the code for this further you can find it in this Github repository.

Thursday, 27 March 2014

Another Look at Unit Testing Umbraco Surface Controllers (Part 1)

When coming to work with Umbraco as an MVC developer one feature that is immediately familiar and comfortable to work with is surface controllers. Unfortunately, unlike standard MVC controllers, they aren't straightforward to test. In this blog post I'm going to look to start from scratch, uncover the issues and see what options we have to get around this.

Failing test attempt

Starting with a simple example taken from the Umbraco documentation, this surface controller action very simply handles a form post.

The view model:

public class CommentViewModel
{
    [Required]
    public string Name { get; set; }

    [Required]
    public string Email { get; set; }

    [Required]
    [Display(Name = "Enter a comment")]
    public string Comment { get; set; }
}

The view and the form in a partial:

@if (TempData["CustomMessage"] != null)
{
    

@TempData["CustomMessage"].ToString()

} @Html.Partial("_CommentForm", new SurfaceControllerUnitTests.Models.CommentViewModel()) ... @model SurfaceControllerUnitTests.Models.CommentViewModel @using (Html.BeginUmbracoForm("CreateComment", "BlogPostSurface")) { @Html.EditorFor(x => Model) <input type="submit" /> }

And the controller:

public class BlogPostSurfaceController : Umbraco.Web.Mvc.SurfaceController
{
    [HttpPost]
    public ActionResult CreateComment(CommentViewModel model)
    {    
        if (!ModelState.IsValid)
        {
            return CurrentUmbracoPage();
        }

        TempData.Add("CustomMessage", "Thanks for your comment.");

        return RedirectToCurrentUmbracoPage();
    }
}

Based on that we can write this test:

[TestMethod]
public void CreateComment_WithValidComment_RedirectsWithMessage()
{
    // Arrange
    var controller = new BlogPostSurfaceController();
    var model = new CommentViewModel
    {
        Name = "Fred",
        Email = "fred@freddie.com",
        Comment = "Can I test this?",
    };

    // Act
    var result = controller.CreateComment(model);

    // Assert
    Assert.IsNotNull(result);
}

Which... fails, with the exception:

System.ArgumentNullException: Value cannot be null.
Parameter name: umbracoContext
Result StackTrace: 
at Umbraco.Web.Mvc.PluginController..ctor(UmbracoContext umbracoContext)
   at Umbraco.Web.Mvc.SurfaceController..ctor()
   at SurfaceControllerUnitTests.Controllers.BlogPostSurfaceController..ctor()

Avoiding the issue

One way to approach this is really to avoid the issue which I'm blogged about briefly before. To move the logic the controller performs out into a separate class that we reference from the controller, that doesn't have an Umbraco dependency. We can then test that. Leaving behind a controller that's so thin there's really little value in testing it.

Now clearly the controller I already have is pretty thin, but sometimes the logic here that goes into the handler class can get quite involved - a registration form that needs validation, date checks, CAPTCHA checks etc. So I've found this a reasonable approach.

The new handler class looks like this:

public class BlogPostSurfaceControllerCommandHandler
{
    public ModelStateDictionary ModelState { get; set; }

    public TempDataDictionary TempData { get; set; }

    public bool HandleCreateComment(CommentViewModel model)
    {
        if (!ModelState.IsValid)
        {
            return false;
        }

        TempData.Add("CustomMessage", "Thanks for your comment.");
        return true;
    }
}

With the controller becoming like this (or you could dependency inject the handler instance):

public class BlogPostSurfaceController : Umbraco.Web.Mvc.SurfaceController
{
    BlogPostSurfaceControllerCommandHandler _commandHandler;

    public BlogPostSurfaceController()
    {
        _commandHandler = new BlogPostSurfaceControllerCommandHandler();
        _commandHandler.ModelState = ModelState;
        _commandHandler.TempData = TempData;
    }

    [HttpPost]
    public ActionResult CreateComment(CommentViewModel model)
    {
        if (!_commandHandler.HandleCreateComment(model))
        {
            return CurrentUmbracoPage();
        }

        return RedirectToCurrentUmbracoPage();
    }
}

And the test for the handler looks like this - which, as expected, now comes out green.

[TestClass]
public class BlogPostSurfaceControllerCommandHandlerTests
{
    [TestMethod]
    public void CreateComment_WithValidComment_ReturnsTrueWithMessage()
    {
        // Arrange
        var handler = new BlogPostSurfaceControllerCommandHandler();
        handler.ModelState = new ModelStateDictionary();
        handler.TempData = new TempDataDictionary();
        var model = new CommentViewModel
        {
            Name = "Fred",
            Email = "fred@freddie.com",
            Comment = "Can I test this?",
        };

        // Act
        var result = handler.HandleCreateComment(model);

        // Assert
        Assert.IsTrue(result);
        Assert.IsNotNull(handler.TempData["CustomMessage"]);
    }
}

Using Umbraco's test classes

So far have only read about this option - so will come back to it in part 2 of this series of posts..

Wednesday, 26 March 2014

Unit testing Umbraco IPublishedContent with Microsoft Fakes

I've recently been working on a package for Umbraco called Umbraco Mapper. It's intention is to support development of MVC based Umbraco applications by providing a simple means of mapping Umbraco content to custom view models. You can read more about it on the GitHub page.

For a few weeks now I've on and off tackled a problem I've found with unit testing part of the functionality. Umbraco content is represented in the front-end of the application by an interface IPublishedContent. What I'm looking to do is to create an instance of IPublishedContent with some known properties, map it to a custom view model class using a method of the mapper, and assert the results are as expected.

First failed attempt... using mocks

In this situation where I have a third party class to instantiate, the first tool I turn to is moq. This is a great little library for unit testing when you have an interface to work with - you can mock an instance that implements this interface and provide known values and functionality for properties and methods. Given those values, you can then action your test and assert the expected outcomes.

Here's a simple mock of IPublished content:

private static IPublishedContent MockIPublishedContent()
{
    var mock = new Mock<IPublishedContent>();
    mock.Setup(x => x.Id).Returns(1000);
    mock.Setup(x => x.Name).Returns("Test content");
    mock.Setup(x => x.CreatorName).Returns("A.N. Editor");
    return mock.Object;
}

And a test that uses that mock:

[TestMethod]
public void UmbracoMapper_MapFromIPublishedContent_MapsNativePropertiesWithMatchingNames()
{
    // Arrange
    var model = new SimpleViewModel();
    var content = MockIPublishedContent();
    var mapper = new UmbracoMapper();

    // Act
    mapper.Map(content, model);

    // Assert
    Assert.AreEqual(1000, model.Id);
    Assert.AreEqual("Test content", model.Name);
}

So far so good, but here I'm only testing properties of IPublishedContent. Of more value in the mapping tool is the mapping of the Umbraco document type properties, which are accessible via a method GetPropertyValue(alias). This makes calls various levels into the Umbraco code base, eventually leading to a null reference exception I believe due to missing a context of some sort.

Now I can add that method to the mock easily enough...

    mock.Setup(x => x.GetPropertyValue(It.IsAny<string>()))
      .Returns((string alias) => MockIPublishedContentProperty(alias));

... but unfortunately this doesn't work, as GetPropertyValue is an extension method that moq is unable to handle. The code compiles, but at runtime you'll get an error and this test will fail.

public void UmbracoMapper_MapFromIPublishedContent_MapsCustomPropertiesWithMatchingNames()
{
    // Arrange
    var model = new SimpleViewModel3();
    var content = MockIPublishedContent();
    var mapper = new UmbracoMapper();

    // Act
    mapper.Map(content, model);

    // Assert
    Assert.AreEqual("This is the body text", model.BodyText);
}

Second failed attempt... using stubs

My next attempt was to create my own class, StubPublishedContent that implements the IPublishedContent interface. Instead of mocking the properties and methods I created stub instantiations of them to just return constant values or simple variations based on inputs.

Of course as I found above, GetPropertyValue(alias) isn't defined on the IPublishedContent interface - but I figured if I just create a method with that signature, given instance methods take precedence over extension methods with the same signature, maybe it'll use my implementation at runtime? Well, no. Confused me for a bit but the citizens of stackoverflow set me straight.

And success... using Microsoft's Fakes

Microsoft Fakes offers a means of replacing certain method calls within any referenced assembly at runtime. To do this you find the assembly that your method is in - in my case umbraco.dll - right-click on the reference and select Add Fakes Assembly.

Once that's complete I could re-write my failing test above like this:

[TestMethod]
public void UmbracoMapper_MapFromIPublishedContent_MapsCustomPropertiesWithMatchingNames()
{
    // Using a shim of umbraco.dll
    using (ShimsContext.Create())
    {
        // Arrange
        var model = new SimpleViewModel3();
        var mapper = new UmbracoMapper();
        var content = new StubPublishedContent();

        // - shim GetPropertyValue (an extension method on IPublishedContent in Umbraco.Web.PublishedContentExtensions)
        Umbraco.Web.Fakes.ShimPublishedContentExtensions.GetPropertyValueIPublishedContentStringBoolean =
            (doc, alias, recursive) =>
            {
                switch (alias)
                {
                    case "bodyText":
                        return "This is the body text";
                    default:
                        return string.Empty;
                }                        
            };

        // Act
        mapper.Map(content, model);

        // Assert
        Assert.AreEqual(1000, model.Id);
        Assert.AreEqual("Test content", model.Name);
        Assert.AreEqual("This is the body text", model.BodyText);
    }
}

I'm replacing the call to GetPropertyValue(alias, recursive) at runtime with my own function, that uses a simple switch statement to return the appropriate document type property value. And at last... a green test!

Monday, 17 March 2014

Async and await with external web resources

Async and await are two keywords that have been available in the C# for a while now. And so whilst probably a bit late to the party thought would share how I've used them for a recent feature, in case they are also new to others.

They allow you to write async code much more easily than was previously possible, which should allow applications to scale better - particularly when there's a need to call out to slow running processes (e.g. external web services), or really anything that is "IO bound" (like a database call.

So for example you could speed up a web request that requires two calls to two long running processes by running them in parallel on different threads and then waiting for both to return. If they both take 1 second your result can return in 1 second instead of 2.

But even with just one long running process in the request, it's still useful, as whilst that process is running the thread serving the request is no longer blocked, and can return to the pool to process other requests. Meaning IIS under load doesn't run out of threads and stop serving content even for simpler or static pages that don't require access to the resource.

Here's the before. I have a controller action that calls up to a service layer to get and parse content from an RSS feed. It ends up at this helper method:

private List<ExternalContentItem> GetItemsFromExternalSource(string url, string category)
 {
    var feedXML = XDocument.Load(url);
    return _rssParser.GetItemsFromXml(feedXML, category).ToList();
 }

And the after. Various long-running methods in .Net 4.5 have been updated to have async versions. XDocument.Load isn't one of them, but if you drop down to the HttpClient you can do this:

private async Task<List<ExternalContentItem>> GetItemsFromExternalSourceAsync(string url, string category)
{
    var httpClient = new HttpClient();
    var response = await httpClient.GetAsync(url).ConfigureAwait(false);
    if (response.StatusCode == HttpStatusCode.OK)
    {
        var stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false);

        var feedXML = XDocument.Load(stream);
        return _rssParser.GetItemsFromXml(feedXML, category).ToList();
    }

    return new List<ExternalContentItem>();
}

To briefly explain the key points:

  • The async keyword does nothing other than to allow you to use await.
  • Await tells the compiler to convert this into async code, so the thread is returned to the pool and code resumes once the call is complete.
  • Your return type needs to change to be a Task of whatever you actually want back.
  • Your calling code can get the result by calling the .Result property on the task.
  • Those ConfigureAwait(false) calls are apparently necessary to avoid deadlocks. They aren't needed if you are using async code throughout the request, but as I'm just amending this method I'm led to believe they are needed here.
  • One other oddity is exception handling. Any exceptions get aggregated up into a new exception type, which is a bit harder to deal with. That's why I've added that check on the status code rather than using a try/catch in the calling code which I had before.

Note important point that this code won't actually run any faster. It's just that it won't block whilst it's waiting.

For this site, this was probably just an academic exercise - I have caching in place too so these calls are very few and far between. But I think this is becoming more and more of an important technique to be aware of.

With MVC we can now create async controllers and with Entity Framework 6 there will also be asynchronous data access. Which means you can be async all the way up and down, and given that, it'll probably be a question of "why not?" rather than "why?" to use them. I can see in the not too distant future it being just the default way to do things.

Saturday, 8 February 2014

Unit Testing Umbraco Surface Controllers

Unit testing Umbraco surface controllers isn't straightforward, due to issues with mocking or faking dependencies. There has been some discussion about it and there are some workarounds regarding using certain test base classes. But in general it's not an easy thing to do, at least at the moment.

Another approach (or workaround) for this is to move the thing you are trying to test outside of the controller and into another class - that itself depends only on standard ASP.Net MVC. Test that instead, leaving your controller so simple that in itself there remains little value in testing it directly.

As an example, I had this to test:

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult SignUp(NewsletterSignUpModel model)
        {
            if (ModelState.IsValid)
            {
                // Model valid so sign-up email address
                var result = _newsletterSignUpService.SignUp(model.Email, model.FirstName, model.LastName);
                TempData["SignUpResult"] = result;
                return RedirectToCurrentUmbracoPage();
            }

            return CurrentUmbracoPage();
        }

Pretty simple registration form. Maybe not much value in testing really, but as I've found these can get quite complex with a lot of logic around validation, age checks, CAPTCHAs etc. So it seems worthwhile to be able to do this.

You can see it has a dependency on a "NewsletterSignUpService" which does the actual signing up of the user (to MailChimp in this case). An instance of this service is provided to the controller via the constructor using dependency injection.

In order to test this, I modified the code to create another class that contains much of this logic - the only difference is it returns a boolean result rather than the Umbraco custom ActionResults such as RedirectToCurrentUmbracoPage:

public class NewsletterSignUpPageControllerCommandHandler : INewsletterSignUpPageControllerCommandHandler
{
    private readonly INewsletterSignUpService _newsletterSignUpService;

    public NewsletterSignUpPageControllerCommandHandler(INewsletterSignUpService newsletterSignUpService)
    {
        _newsletterSignUpService = newsletterSignUpService;
    }

    public bool HandleSignUp(NewsletterSignUpModel model, ModelStateDictionary modelState, TempDataDictionary tempData)
    {
        if (modelState.IsValid)
        {
            // Model valid so sign-up email address
            var result = _newsletterSignUpService.SignUp(model.Email, model.FirstName, model.LastName);
            tempData["SignUpResult"] = result;
            return true;
        }

        return false;
    }
}

You can see the dependency on the service is now in here, and I can then change my controller to depend instead on this new class:

        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult SignUp(NewsletterSignUpModel model)
        {
            if (_newsletterSignUpCommandHandler.HandleSignUp(model, ModelState, TempData))
            {
                return RedirectToCurrentUmbracoPage();
            }

            return CurrentUmbracoPage();
        }

And having done that I can then write the tests on the command handler class (mocking the dependent service layer to return the responses I want to simulate), e.g.:

#region Newsletter sign-up tests

[TestMethod]
public void NewsletterSignUpPageCommandHandler_SignUpPostInvalid_ReturnsFalse()
{
    // Arrange
    var model = new NewsletterSignUpModel
    {
        Email = string.Empty,
    };
    var modelStateDictionary = new ModelStateDictionary();
    modelStateDictionary.AddModelError("Email", "Email is required.");
    var tempDataDictionary = new TempDataDictionary();
    var handler = new NewsletterSignUpPageControllerCommandHandler(MockNewsletterSignUpService());

    // Act
    var result = handler.HandleSignUp(model, modelStateDictionary, tempDataDictionary);

    // Assert
    Assert.IsFalse(result);
    Assert.AreEqual(0, tempDataDictionary.Keys.Count);
}

[TestMethod]
public void NewsletterSignUpPageCommandHandler_SignUpPostValid_ReturnsTrue()
{
    // Arrange
    var model = new NewsletterSignUpModel
    {
        Email = "fred@test.com",
    };
    var modelStateDictionary = new ModelStateDictionary();
    var tempDataDictionary = new TempDataDictionary();
    var handler = new NewsletterSignUpPageControllerCommandHandler(MockNewsletterSignUpService());

    // Act
    var result = handler.HandleSignUp(model, modelStateDictionary, tempDataDictionary);

    // Assert
    Assert.IsTrue(result);
    Assert.IsNotNull(tempDataDictionary["SignUpResult"]);
    Assert.AreEqual(NewsletterSignUpResult.Success, (NewsletterSignUpResult)tempDataDictionary["SignUpResult"]);
}

[TestMethod]
public void NewsletterSignUpPageCommandHandler_SignUpPostValidButAlreadySignedUp_ReturnsTrue()
{
    // Arrange
    var model = new NewsletterSignUpModel
    {
        Email = "fred2@test.com",
    };
    var modelStateDictionary = new ModelStateDictionary();
    var tempDataDictionary = new TempDataDictionary();
    var handler = new NewsletterSignUpPageControllerCommandHandler(MockNewsletterSignUpService());

    // Act
    var result = handler.HandleSignUp(model, modelStateDictionary, tempDataDictionary);

    // Assert
    Assert.IsTrue(result);
    Assert.IsNotNull(tempDataDictionary["SignUpResult"]);
    Assert.AreEqual(NewsletterSignUpResult.FailedAlreadySignedUp, (NewsletterSignUpResult)tempDataDictionary["SignUpResult"]);
}

#endregion

#region Mocks

private static INewsletterSignUpService MockNewsletterSignUpService()
{
    var mock = new Mock<INewsletterSignUpService>();
    mock.Setup(x => x.SignUp(It.IsAny<string>(), It.IsAny(), It.IsAny())).Returns(NewsletterSignUpResult.Success);
    mock.Setup(x => x.SignUp(It.Is<string>(y => y == "fred2@test.com"), It.IsAny(), It.IsAny())).Returns(NewsletterSignUpResult.FailedAlreadySignedUp);
    return mock.Object;
}

#endregion

Quite nice I think - means we can have testable code, and also follows the good design principles of having "thin" controllers with work delegated to small, defined classes.

Monday, 6 January 2014

JSON object versus array gotcha

Pulled a little of what's left of my hair out with this little gotcha looking at an issue with Umbraco.

I was looking to add a small feature to allow you to re-order a list.  Umbraco (version 7) uses angularjs in the back-office.  Updating this controller and view code was very straightforward to add a couple of buttons to move the list items up and down.  However although I could see the order was properly being saved to the database, the retrieved list remained in the order that the items were created.

Using the Chrome console network tab I could see that the JSON response was coming back from the server in the correct order:

... key":"items","value":{"10":"aaa","7":"bbb","8":"ccc"} ...

However as soon as this materialised in code, the order was lost:

... key":"items","value":{"7":"bbb","8":"ccc","10":"aaa"} ...

The reason for loss of hair was that I was sure that somewhere in the code I'd find that the order was being changed, but it wasn't this.  It was because the JSON is being structured as an object rather than an array, and it seems that if you do this, you can't guarantee that the order of the elements would be the same.

Ideally this would be changed to an array, but a less instrusive change for a code base that's obviously not my own was to add the sort order to the object:

... key":"items","value":{"10": {"value":"aaa","sortOrder":1}, "7":  {"value":"bbb","sortOrder":2},"8":  {"value":"ccc","sortOrder":3}} ...

This meant it could then be re-sorted on the client:

var items = [];
for (var i in $scope.model.value) { 
  items.push({
    value: $scope.model.value[i].value,
    sortOrder: $scope.model.value[i].sortOrder,
    id: i
  });
}
items.sort(function (a, b) { return (a.sortOrder > b.sortOrder) ? 1 : ((b.sortOrder > a.sortOrder) ? -1 : 0); });

Wednesday, 26 June 2013

Umbraco Core Contributions Via Github

Umbraco CMS have recently moved their source code from CodePlex to Github which has triggered a flurry of contributions in the form of pull requests. Previously I had made a few small contributions to the project whilst on Codeplex and have now submitted some to Github.

Whilst GitHub is very easy to use and has excellent documentation I managed to get in a bit of a tangle trying to manage different issues, so mostly for my own future benefit, am noting two working processes here. If anyone else reads, first port of call should be the Umbraco contribution documentation and GiHub's own help pages on forking and pull requests.

One issue is that like other source control hosts, only one pull request can be submitted per branch. In order to keep things simple for the person on the other end, it makes sense not to combine several issues into one pull request.

Hence coding the first issue in master branch, pull requesting, and then trying to move onto a second in master branch will lead to problems. Really you'd have to wait until the first issue is accepted or rejected before working on a second issue in the same branch, which obviously isn't ideal.

Another issue is that, unlike Codeplex, you can only have one fork of a main repository - so you can't create a fork for each issue you work on.

Here are two methods to work around these two restrictions:

What To Do Right From The Start

This has been added to the Umbraco contribution page, so you can read it there under the section "When you're working on multiple issues at the same time".

Or Another Way... That I'm Currently Doing Given I Used Master Branch For My First Pull Request

Assuming you have a fork set up, and have cloned it locally, set up a remote of your fork called origin and one of the main repository called upstream

  1. Code and commit your work in your local master branch
  2. Create a new branch based on the upstream master branch (or other branch if more appropriate
        git branch [branch name] upstream/master
    
  3. Pull out just the commit(s) you want into your branch that you will create the pull request from. You can find the SHA of each commit using the Git GUI interface.
        git checkout [branch name]
        git cherry-pick [SHA of commit]
    
  4. Push your changes to your fork
        git push origin [branch name]
    
  5. Create a pull request from your branch into the master (or other if more appropriate) branch of the main repostory. It will now contain just the commits you want.