Published on 8/16/2019

Now let’s move back to the syndication service. We have a bit to do to get from the point where we implement the new IBlogSyndicationService abstraction to where we’re calling into the IBlogSyndication abstractions that we use for the different providers.

Re-plan

So, since we changed the implementation a bit in part 9, we need to review our plan and make sure that our design and architecture is still aligned with our code and points of abstractions.

This picture mostly looks the same as the last one in part 7. The black in this picture is what we’ve already done, or mostly done and where we have well defined abstractions. Those marked or written in purple still needs to be implemented or defined. Additionally, I’ve scratched out elements of our current implementation and corrected it in purple where we had deviated from our plan from part 7. We did not include taking in the submitted list of syndication texts at the Publish() method, and so we injected the configured set of syndications straight into the BlogSyndicationService. Previously we came up with the idea that the front-end will render the required elements dictated by config, and then the submitted syndication texts will be PUT together with the title and that is what will drive the for-loop in the service.

Adjust the tests

We already have tests for publish, so let’s adjust them to make sure we assert for the correct behavior. That behavior is that the Publish() method should accept a collection of syndication texts, and pass it along to the Syndicate() method.

[TestMethod]
public async Task Publish_EnqueueSyndications()
{
    //arrange
    string title = "hello_test";
    Blog blog = new Blog() { Title = "Hello Test!" };
    List<SyndicationText> syndicationTexts = new List<SyndicationText>()
    {
        new SyndicationText() { Name = "Twitter", Text = "Hello #World!" },
        new SyndicationText() { Name = "Facebook", Text = "Today I write Hello World!" },
    };
    _dbAdaptorMock.Setup(x => x.Read(title))
        .ReturnsAsync(blog);

    //act
    await Service.Publish(title, syndicationTexts);

    //assert
    _blogSyndicationServiceMock.Verify(x => x.Syndicate(blog, syndicationTexts));
}

I only added it to this enqueue unit test since it’s the only one where we verify the call to the syndication service. And in order to compile with this change (and pass the unit test) we need to amend and pass the collection of SyndicationText items in quite a few places. Were I wasn’t interested in it yet (or even at all) I simply passed null.

Now that we take it in at the BlogService class and verify that we pass it on to the BlogSyndicationService class, let’s move on to the latter and adjust those tests for dealing with this list. So what we need here is to match the list of syndications that we’re now taking in with the list of configured syndications injected through the config. We only want to process syndications that we received text for, but we need to match it to the config to get any specific metadata or other items related to that syndication provider. This means that our loop here should not iterate over the configured items anymore, but instead the supplied items. How do we test for that? In the test arrangement we purposefully miss-align the supplied set of syndications with the configured set of syndications. We can then assert that, given the arranged scenario, only a specific result should be verified.

[TestMethod]
public async Task Syndicate_Verify()
{
    //arrange
    string title = "Hello Test!";
    Blog blog = new Blog() { Title = title };
    List<SyndicationText> syndicationTexts = new List<SyndicationText>()
    {
        new SyndicationText() { Name = "Twitter", Text = "Hello #World!" },
        new SyndicationText() { Name = "Facebook", Text = "Today I hello worlded"}
    };
    BlogSyndicationOptionCollection syndicationConfig = new BlogSyndicationOptionCollection()
    {
        new BlogSyndicationOption() { Provider = "RSS" },
        new BlogSyndicationOption() { Provider = "Twitter" },
        new BlogSyndicationOption() { Provider = "Facebook" }
    };
    Mock<IBlogSyndication> syndicationMock = new Mock<IBlogSyndication>();
    _blogSyndicationOptionsMock.SetupGet(x => x.CurrentValue)
        .Returns(syndicationConfig);
    _blogSyndicationFactoryMock.Setup(x => x.GetInstance(It.IsAny<string>()))
        .Returns(syndicationMock.Object);

    //act
    await Service.Syndicate(blog, syndicationTexts);

    //assert
    _blogSyndicationFactoryMock.Verify(x => x.GetInstance("Twitter"));
    _blogSyndicationFactoryMock.Verify(x => x.GetInstance("Facebook"));
    _blogSyndicationQueueMock.Verify(x => x.Enqueue(syndicationMock.Object), Times.Exactly(2));
    syndicationMock.VerifySet(x => x.Blog = blog, Times.Exactly(2));
    syndicationMock.VerifySet(x => x.Config = syndicationConfig[1]);
    syndicationMock.VerifySet(x => x.Config = syndicationConfig[2]);
}

This test has changed significantly, so let’s break it down. I added the new collection of SyndicationText items. This represents the list of items that the user filled in, that was passed at the PUT method and then on to this service by the Publish() method. Then I extracted the BlogSyndicationOptionCollection from the mock declaration and added Facebook to the set of configured syndication options. There is now three configured items, but only two supplied items. I passed the syndication texts collection to the Syndicate() method. I also changed the assert section to now verify on the factory only Twitter and Facebook (previously RSS and Twitter) since those are the two supplied syndication texts. Then I also added two additional verify calls to the IBlogSyndication mock object to assert that the config was set twice, once for Twitter and once for Facebook (elements 1 and 2 in the configuration collection). To compile this I added a config element on the IBlogSyndication abstraction.

Adjust the implementation

Now that we’ve established our red phase, let’s pass this changed unit test. We need to change the projection source, and we need to change the assignment before we enqueue.

public async Task Syndicate(Blog blog, IEnumerable<SyndicationText> syndicationTexts)
{
    if (_syndicationCollection == null || _syndicationCollection.Count == 0)
    {
        _logger?.LogWarning("No syndications configured for processing");
    }
    else
    {
        IEnumerable<Task> syndicationTasks = syndicationTexts.Select(t => SyndicateItem(t, blog));
        await Task.WhenAll(syndicationTasks);
    }
}

private async Task SyndicateItem(SyndicationText text, Blog blog)
{
    BlogSyndicationOption config = _syndicationCollection.SingleOrDefault(o => o.Provider == text.Name);
    IBlogSyndication syndication = _blogSyndicationFactory.GetInstance(config.Provider);
    syndication.Blog = blog;
    syndication.Config = config;
    await _blogSyndicationQueue.Enqueue(syndication);
}

The subtlety of the changes here is actually pretty amazing. The projection in the .Select() call was changed to operate on the SyndicationText collection instead, and it’s hardly noticeable. Then the delegate we use in the project was obviously changed to accept the text item instead, and find the corresponding configuration item for it. It then sets it on the result from the factory. But there is one problem now.

Concept: Defensive Code

That’s just guards, right? Specifically, you might associate this with methods named Guard() that is basically validation at the entry points of some other methods. You’re not wrong, but writing good defensive good is very hard, and I’ve seen the concept being abused and beaten to death in code so much that I’ve come to dislike the Guard() pattern immensely. Defensive code’s primary purpose is to inform, rather than to block. And I think the bad sort manifests itself from fear of failing:

"God forbid the stack trace shows the error happened in the code I wrote!"

And because of this fear most people write defensive code to insulate their work from situations not in their control. Configuration is a very good example of this. Pretend you have to rely on some configuration to perform a task in your code. But what if that configuration is incorrect or completely missing because some devops guy messed it up in the last deployment? You don’t want to be called over the weekend when the operations people do that first smoke test after go-live. So of course, I’ve seen loads of do nothing and exit code in my life. And you know what? Now the entire team is going to be called in on the weekend because no-one knows what’s going on.

It’s much better to do the checks and inform about the problem (by throwing an appropriate exception, and also explicitly logging out the problem) so that it’s quick and easy to see and understand what caused it.

“Oh right! We forgot the config!”

When you inform properly, another person has the ability to help themselves instead of having to call and ask you.

Concept: Failing Fast

Hand in hand with defensive code and error handling strategies we discussed in part 4, is the idea of failing fast. I’ve touched on it before in various parts of this series, mostly pointing out how the .NET libraries and APIs immediately informs us of the mistakes we’re making when dealing with it by throw appropriate exceptions. There’s no point in trying to deal with input errors or state errors while still attempting to generate a meaningful result. And it’s almost never a good idea to catch the error and then simply swallow it. If you do, justify it and unit test for it specifically! A much better strategy is to formulate a good informative error message and throw an appropriate exception.

So, in this private delegate method we use the LINQ extension Single() to get the corresponding config item. But what if it’s not there? Or what if there’s more than one config element with the same Provider value? Single() will throw an InvalidOperationException in both those cases. Someone looking at the event log will remain uninformed when seeing this generic “Sequence contains no elements” exception in the logs or the exception page. Here we should take the opportunity to handle this specific exception type and report on it properly.

Test for and handle the exception

So just like for empty config, let’s build another exception handling unit test for when the config is duplicated.

[TestMethod]
public async Task Syndicate_DuplicateConfig_Log()
{
    //arrange
    string title = "Hello Test!";
    Blog blog = new Blog() { Title = title };
    List<SyndicationText> syndicationTexts = new List<SyndicationText>()
    {
        new SyndicationText() { Name = "Twitter", Text = "Hello #World!" },
        new SyndicationText() { Name = "Facebook", Text = "Today I hello worlded"}
    };
    BlogSyndicationOptionCollection syndicationConfig = new BlogSyndicationOptionCollection()
    {
        new BlogSyndicationOption() { Provider = "Twitter" },
        new BlogSyndicationOption() { Provider = "Twitter" },
        new BlogSyndicationOption() { Provider = "Facebook" }
    };
    Mock<IBlogSyndication> syndicationMock = new Mock<IBlogSyndication>();
    _blogSyndicationOptionsMock.SetupGet(x => x.CurrentValue)
        .Returns(syndicationConfig);
    _blogSyndicationFactoryMock.Setup(x => x.GetInstance(It.IsAny<string>()))
        .Returns(syndicationMock.Object);

    //act
    await Service.Syndicate(blog, syndicationTexts);

    //assert
    _blogSyndicationFactoryMock.Verify(x => x.GetInstance("Facebook"));
    _blogSyndicationQueueMock.Verify(x => x.Enqueue(syndicationMock.Object));
    syndicationMock.VerifySet(x => x.Blog = blog);
    syndicationMock.VerifySet(x => x.Config = syndicationConfig[2]);

    _loggerMock.Verify(x => x.Log(LogLevel.Error, It.IsAny<EventId>(), It.IsAny<object>(), It.IsAny<Exception>(), It.IsAny<Func<object, Exception, string>>()));
    var state = _loggerMock.Invocations[0].Arguments[2] as IReadOnlyList<KeyValuePair<string, object>>;
    Assert.IsTrue(((string)state[0].Value).Contains("Twitter"));
}

This test has essentially the same arrangement as the verify test, with duplicated config for Twitter. Then we assert that we log an error, and that the content of that error contains the name “Twitter”. But we also include the asserts that the syndication to Facebook occurred. Why is this? Recall that we are now dealing with an automated process. In the same way that we only logged the warning when no config was present (see part 8), we’re swallowing the exception from Single(), log an error, and move on. We don’t want to stop the syndications that do work from actually working. So at this point I can imagine you’re frowning.

Am I contradicting myself now?

Well, sort of, but I’m justifying it and I’m unit testing for it specifically. Remember that we’re dealing with a collection and an iteration over that collection in an automated process. This process is unattended by a human (apart from the user that clicked the “Publish” button). We can inspect all the required config upfront and decide whether we want to fail the entire thing and let the user know. Or we can process what we can, while we fail fast, log and exit on what we can’t. The former is valid, but typically imposes performance penalties. Just like the model error handling strategy, you will have to perform the validation check for all the entries before processing all the entries (two loops!). And if everything checks out you’ve done unnecessary work. The latter is faster since it handles each iteration in isolation and complete the ones that it can, while aborting the ones that it can’t with sufficient logging (and yes, there are also performance penalties handling the exceptions for specific iterations too).

Of course this doesn’t always apply. Sometimes you are in a “transaction” that has to complete in its entirety or not at all. Then the former implementation is what you need to do. But whatever your implementation is, make sure that you confirm it in unit tests.

Let’s look at my implementation to pass that test.

private async Task SyndicateItem(SyndicationText text, Blog blog)
{
    try
    {
        BlogSyndicationOption config = _syndicationCollection.Single(o => o.Provider == text.Name);
        IBlogSyndication syndication = _blogSyndicationFactory.GetInstance(config.Provider);
        syndication.Blog = blog;
        syndication.Config = config;
        await _blogSyndicationQueue.Enqueue(syndication);
    }
    catch(InvalidOperationException ex)
    {
        _logger?.LogError(ex, $"Config for provider '{text.Name}' not valid.");
    }
}

We also have a case where Single() will throw for no element found, where syndication text was supplied for config that doesn’t exist. Perhaps there’s a bug elsewhere and it manifests itself in your code by supplying syndication text that the user never entered. We don’t need another unit test for this though. We can simply adjust the arrangement and asserts of this existing test (and change its name) to cover this case too.

[TestMethod]
public async Task Syndicate_MessedConfig_Log()
{
    //arrange
    string title = "Hello Test!";
    Blog blog = new Blog() { Title = title };
    List<SyndicationText> syndicationTexts = new List<SyndicationText>()
    {
        new SyndicationText() { Name = "Twitter", Text = "Hello #World!" },
        new SyndicationText() { Name = "Instagram", Text = "Hello World! #program #software #development #blogging" },
        new SyndicationText() { Name = "Facebook", Text = "Today I hello worlded"}
    };
    BlogSyndicationOptionCollection syndicationConfig = new BlogSyndicationOptionCollection()
    {
        new BlogSyndicationOption() { Provider = "Twitter" },
        new BlogSyndicationOption() { Provider = "Twitter" },
        new BlogSyndicationOption() { Provider = "Facebook" }
    };
    Mock<IBlogSyndication> syndicationMock = new Mock<IBlogSyndication>();
    _blogSyndicationOptionsMock.SetupGet(x => x.CurrentValue)
        .Returns(syndicationConfig);
    _blogSyndicationFactoryMock.Setup(x => x.GetInstance(It.IsAny<string>()))
        .Returns(syndicationMock.Object);

    //act
    await Service.Syndicate(blog, syndicationTexts);

    //assert
    _blogSyndicationFactoryMock.Verify(x => x.GetInstance("Facebook"));
    _blogSyndicationQueueMock.Verify(x => x.Enqueue(syndicationMock.Object));
    syndicationMock.VerifySet(x => x.Blog = blog);
    syndicationMock.VerifySet(x => x.Config = syndicationConfig[2]);

    _loggerMock.Verify(x => x.Log(LogLevel.Error, It.IsAny<EventId>(), It.IsAny<object>(), It.IsAny<Exception>(), It.IsAny<Func<object, Exception, string>>()));
    var state = _loggerMock.Invocations[0].Arguments[2] as IReadOnlyList<KeyValuePair<string, object>>;
    Assert.IsTrue(((string)state[0].Value).Contains("Twitter"));
    state = _loggerMock.Invocations[1].Arguments[2] as IReadOnlyList<KeyValuePair<string, object>>;
    Assert.IsTrue(((string)state[0].Value).Contains("Instagram"));
}

Here we assert for logs on both Twitter (duplicate config) and Instagram (no config), but process Facebook.

Conclusion

In this part we revisited our plan and identified additional bits that we skipped. This happens a lot in real life too. Sometimes you miss something in the first planning session, or you consciously decide to iterate over the implementation a few times, introducing more complex implementations for special or specific use cases each time. Regardless, reviewing and re-planning is key to reaffirm your understanding of your implementation goal. We discussed the concepts of writing defensive code and failing fast, and how and why we need to think about our strategies for informing our team members of errors. We discussed throwing or swallowing an error, and we looked at a scenario where each of those strategies are valid.

At some companies there are dedicated production or customer support teams that handle issues, while at others the development team themselves are tasked with this duty. There are pros and cons for both, but the bottom line is this: Handle errors in a way that is transparent and that will inform and help whoever is supporting your code in the wild. If that’s you, then you will help yourself and your team (and they will love you for it). If that’s a support team, then make life easier for them.