Learning TDD by doing: An SPA content finder for Umbraco

Remember: 🔴 red, 🟢 green, ♻️ refactor

For a project that I'm working on, I wanted to support an SPA in an Umbraco application. That means I needed a way to always return the same template, no matter what the URL is. I figured I could do this by creating an Umbraco content finder and so I started this assignment.

Context

A content finder in Umbraco is a class that tries to assign a content item from the backoffice to the current request. You can use it to render specific content items for requests under certain conditions. In my case the condition was as follows: When a user visits a URL that's in the domain of my SPA app, they should always see the content from the domain root.

This piece of code will deal with only one system edge, the Umbraco framework:

Initial attempt

I started out by making an empty implementation of Umbraco's IContentFinder interface:

SPAContentFinder.cs

public class SPAContentFinder : IContentFinder
{
    public Task<bool> TryFindContent(IPublishedRequestBuilder request)
    {
        return Task.FromResult(true);
    }
}

The next step was to find the most simple usecase and write a test for it. "TryFindContent should be able to assign the SPA root content", was what I came up with. My first attempt to write a test looked like this:

SPAContentFinderTests.cs

[Fact]
public async Task ShouldAssignSPARootAsPublishedContent()
{
    // given
    var targetContent = new SPARootPage(new FakePublishedContent(id: 1234), Substitute.For<IPublishedValueFallback>());
    var fakePublishedRequestBuilder = new FakePublishedRequestBuilder(
        domain: new DomainAndUri(
            new Domain(1, "example.com", 1234, null, false, 0),
            new Uri("https://example.com/test")));
    var sut = new SPAContentFinder();

    // when
    await sut.TryFindContent(fakePublishedRequestBuilder);

    // then
    Assert.Equal(targetContent, fakePublishedRequestBuilder.AssignedContent);
}

I didn't feel comfortable with this test. The "given" part didn't read like a sentence and it involved a big deal of useless default data, just so I could create a model to test with. This test was difficult to write (and it isn't even finished yet).

💡 Learning
The initial attempt to write a test didn't work. Dave Farley says that if a test is difficult to write, then your test is telling you that your code is bad. I decided to redesign my code to be easier to test. Do you agree with this decision?

Second attempt: writing my own object

In order to make the test easier to write, I decided to first disregard the Umbraco interface and instead make my own object. This object would reflect how I expected it to work. I started by writing a test:

[Fact]
public void ShouldAssignSPARootAsPublishedContent()
{
    // given
    var targetContent = new SPARootPage(new FakePublishedContent(1234), Substitute.For<IPublishedValueFallback>());
    var contentStore = new FakeContentStore(targetContent);
    var context = new FakeRequestContext(1234);

    // when
    new SPAContentFinderLogic(contentStore, context).AssignContentFromRoot();

    // then
    Assert.Equal(targetContent, context.AssignedContent);
}

This test was already way more comfortable to read. The setup is quite simple (ignoring the target content model) with each required object setup in a single line and all fitting inside one screenwidth.

I created a bare minimum code to make the project compile:

public interface ISPAContentStore
{
    IPublishedContent? Get(int id);
}

public interface ISPARequestContext
{
    int GetDomainContentId();
    void AssignContent(IPublishedContent content);
}

public class SPAContentFinderLogic(ISPAContentStore store, ISPARequestContext context)
{
    public void AssignContentFromRoot()
    { }
}

At this point I could run my test and I finished 🔴 red. I fixed the test with the most straightforward implementation I could think of:

public class SPAContentFinderLogic(ISPAContentStore store, ISPARequestContext context)
{
    public void AssignContentFromRoot()
    {
        var content = store.Get(context.GetDomainContentId());
        context.AssignContent(content);
    }
}

Now the test was 🟢 green. I decided against ♻️ refactoring at this point.

❓ Uncertainty
In order to write this simple code, I had to ignore some nullability warnings. Was I supposed to ignore them? Leaving warnings in my code felt counter-intuïtive, but fixing them at this point seemed against how TDD is supposed to work. What do you think?

There were two more cases that I needed to cover:

• AssignContentFromRoot should not assign non-spa content as root
• AssignContentFromRoot should not assign any content without a domain

I did some small refactors in the process and ended up with this test class:

public class SPAContentFinderTests
{
    private readonly FakePublishedContentStore _store;
    private readonly FakeRequestContext _context;

    public SPAContentFinderTests()
    {
        _store = new FakePublishedContentStore();
        _context = new FakeRequestContext();
    }

    [Fact]
    public void ShouldAssignSPARootAsPublishedContent()
    {
        // given
        var targetContent = new SPARootPage(new FakePublishedContent(1234), Substitute.For<IPublishedValueFallback>());
        _store.Add(targetContent);
        _context.SetDomainContentId(1234);

        // when
        SPAContentFinderLogic.AssignContentFromDomain(_store, _context);

        // then
        Assert.Same(targetContent, _context.AssignedContent);
    }

    [Fact]
    public void ShouldNotAssignOtherRoot()
    {
        // given
        var content = new DetailPage(new TestPublishedContent(1234), Substitute.For<IPublishedValueFallback>());
        _store.Add(content);
        _context.SetDomainContentId(1234);

        // when
        SPAContentFinderLogic.AssignContentFromDomain(_store, _context);

        // then
        Assert.Null(_context.AssignedContent);
    }

    [Fact]
    public void ShouldNotAssignWithoutDomain()
    {
        // given

        // when
        SPAContentFinderLogic.AssignContentFromDomain(_store, _context);

        // then
        Assert.Null(_context.AssignedContent);
    }
}

It produced the following implementation:

public static class SPAContentFinderLogic
{
    public static void AssignContentFromDomain(ISPAContentStore store, ISPARequestContext context)
    {
        if (!context.HasDomainAssigned()) return;

        var contentId = context.GetDomainContentId();
        var content = store.Get(contentId);

        if (content is SPARootPage)
        {
            context.AssignContent(content);
        }
    }
}

✅ Success!
I succesfully wrote my business logic using Test Driven Development. De tested code is simple, easy to read and understand. The tests that accompany the code are simple as well.

Connecting the implementation to Umbraco

What was left was to actually implement IContentFinder. I didn't like the test that I wrote in the first attempt, so I decided against writing unit tests for this part of the code:

public class SPAContentFinder(IUmbracoContextAccessor umbracoContextAccessor)
    : IContentFinder
{
    public Task<bool> TryFindContent(IPublishedRequestBuilder request)
    {
        var context = new RequestBuilderBasedContext(request);
        var store = new CacheBasedContentStore(umbracoContextAccessor.GetRequiredUmbracoContext().Content);

        SPAContentFinderLogic.AssignContentFromDomain(store, context);
        return Task.FromResult(context.IsContentAssigned);
    }

    private class RequestBuilderBasedContext(IPublishedRequestBuilder builder)
        : ISPARequestContext
    {
        public bool IsContentAssigned { get; private set; }

        public void AssignContent(IPublishedContent content)
        {
            builder.SetPublishedContent(content);
            IsContentAssigned = true;
        }

        public int GetDomainContentId()
        {
            if (!builder.HasDomain()) throw new InvalidOperationException("Cannot get the content id, because no domain is assigned to the current request");
            return builder.Domain!.ContentId;
        }

        public bool HasDomainAssigned()
        {
            return builder.HasDomain();
        }
    }

    private class CacheBasedContentStore(IPublishedContentCache? cache)
        : ISPAContentStore
    {
        public IPublishedContent? Get(int id)
        {
            if (cache is null) return null;

            return cache.GetById(id);
        }
    }
}

❓ Uncertainty

I have several uncertainties at this point: How could I test this code, if not with a unit test? This method made me introduce two additional facade objects to abstract Umbraco away. Is this overengineered? My abstraction is leaky, because I still use Umbraco's published content model. Was this a mistake? Could I have done this differently? The single-method static class feels as though it should've just been a method inside the content finder, but if it were in there, it feels like an implementation detail. Was it the right choice to define the business logic in a separate, static, logic class?

Closing thoughts

I feel like I've made a good start. Using the unit tests, I was able to implement my logic in simple small steps. That being said, I feel very uncertain about what I ended up with. Did I test enough? Did I take the right steps? It also feels as if my solution here is somewhat over-engineered. The untested code is still so much larger than the tested code and that seems like a red flag to me. Perhaps there wasn't enough business logic to really justify the abstraction?

What do you think? Did I do a good job at Test Driven Development for this simple exercise? Let me know with a comment! That's it for now, thank you for reading and I hope to see you in my next blog! 😊