Permalink
Browse files

Suppressed exceptions when updating the index.

The motivation is that when a new page is created, the second of two
update operations may fail, which means that the index document's last
link becomes stale. The system is still 'almost consistent' in the sense
that all other links point to correct documents, apart from the last link
in the index document. This link points to an existing document, but that
document is no longer the most recent document.

However, if the update operation of the index throws an exception, a
client may interpret that as a failure to write an event to storage, and
thus may retry the operation. This would be an error, because the event
has been written, and thus, a duplicate event would be written if the
operation is retried.

For that reason, while the first update operation (of the new previous
page) must succeed or throw an exception, the second update (of the index
page) must silently fail if an error occurs.
  • Loading branch information...
ploeh committed Nov 20, 2014
1 parent 369bd18 commit 615cdee2c4d675d412e6669bcc0678655376c4d1
Showing with 70 additions and 2 deletions.
  1. +68 −1 AtomEventStore.UnitTests/AtomEventObserverTests.cs
  2. +2 −1 AtomEventStore/AtomEventObserver.cs
@@ -9,6 +9,7 @@
using Ploeh.AutoFixture.Xunit;
using Xunit;
using Ploeh.AutoFixture;
using Moq;
namespace Grean.AtomEventStore.UnitTests
{
@@ -201,6 +202,63 @@ public class AtomEventObserverTests
Assert.Equal(2, writtenIds.Count(id => sut.Id == id));
}
[Theory]
[InlineAutoAtomData(1, AtomEventWriteUsage.AppendAsync)]
[InlineAutoAtomData(1, AtomEventWriteUsage.OnNext)]
[InlineAutoAtomData(2, AtomEventWriteUsage.AppendAsync)]
[InlineAutoAtomData(2, AtomEventWriteUsage.OnNext)]

This comment has been minimized.

Show comment
Hide comment
@sgryt

sgryt Nov 20, 2014

Contributor

IMO, the purpose of this test would stand out clearer if the Last part of the method name was changed to Index.

@sgryt

sgryt Nov 20, 2014

Contributor

IMO, the purpose of this test would stand out clearer if the Last part of the method name was changed to Index.

This comment has been minimized.

Show comment
Hide comment
@ploeh

ploeh Nov 20, 2014

Contributor

So, WriteMoreThanPageSizeDoesNotThrowWhenIndexWriteFails?

@ploeh

ploeh Nov 20, 2014

Contributor

So, WriteMoreThanPageSizeDoesNotThrowWhenIndexWriteFails?

This comment has been minimized.

Show comment
Hide comment
@sgryt

sgryt Nov 20, 2014

Contributor

Yes :)

@sgryt

sgryt Nov 20, 2014

Contributor

Yes :)

This comment has been minimized.

Show comment
Hide comment
@ploeh

ploeh Nov 20, 2014

Contributor

OK; rename pushed to PR branch.

@ploeh

ploeh Nov 20, 2014

Contributor

OK; rename pushed to PR branch.

public void WriteMoreThanPageSizeDoesNotThrowWhenLastWriteFails(
int pageCount,
AtomEventWriteUsage usage,
[Frozen(As = typeof(ITypeResolver))]TestEventTypeResolver dummyResolver,
[Frozen(As = typeof(IContentSerializer))]XmlContentSerializer dummySerializer,
[Frozen]Mock<IAtomEventStorage> storeStub,
AtomEventsInMemory innerStorage,
AtomEventWriterFactory<XmlAttributedTestEventX> writerFactory,
AtomEventObserver<XmlAttributedTestEventX> sut,
Generator<XmlAttributedTestEventX> eventGenerator)
{
// Fixture setup
storeStub
.Setup(s => s.CreateFeedReaderFor(It.IsAny<Uri>()))
.Returns((Uri u) => innerStorage.CreateFeedReaderFor(u));
storeStub
.Setup(s => s.CreateFeedWriterFor(It.IsAny<AtomFeed>()))
.Returns((AtomFeed f) => innerStorage.CreateFeedWriterFor(f));
var writer = writerFactory.Create(usage);
/* Write some pages full. */
var events = eventGenerator.Take(sut.PageSize * pageCount).ToList();
events.ForEach(e => writer.WriteTo(sut, e));
/* Find the index. */
var writtenFeeds = innerStorage.Feeds.Select(ParseAtomFeed);
var index = FindIndex(writtenFeeds, sut.Id);
/* Configure storage to fail when writing the index, but not for
* other documents. */
storeStub
.Setup(s => s.CreateFeedWriterFor(
It.Is<AtomFeed>(f => f.Id != index.Id)))
.Returns((AtomFeed f) => innerStorage.CreateFeedWriterFor(f));
storeStub
.Setup(s => s.CreateFeedWriterFor(
It.Is<AtomFeed>(f => f.Id == index.Id)))
.Throws(new Exception("On-purpose write failure."));
// Exercise system
var @event = eventGenerator.First();
writer.WriteTo(sut, @event);
// Verify outcome
writtenFeeds = innerStorage.Feeds.Select(ParseAtomFeed);
var lastPage = FindLastPage(writtenFeeds, sut.Id);
Assert.True(
lastPage.Entries.Select(e => e.Content.Item).Any(@event.Equals),
"Last written event should be present.");
// Teardown
}
[Theory]
[InlineAutoAtomData(AtomEventWriteUsage.AppendAsync)]
[InlineAutoAtomData(AtomEventWriteUsage.OnNext)]
@@ -388,7 +446,7 @@ public class AtomEventObserverTests
var writtenFeeds = storage.Feeds.Select(ParseAtomFeed);
var firstPage = FindFirstPage(writtenFeeds, sut.Id);
var nextPage = FindNextPage(firstPage, writtenFeeds);
var previousLink =
var previousLink =
nextPage.Links.SingleOrDefault(l => l.IsPreviousLink);
Assert.NotNull(previousLink);
Assert.Equal(
@@ -422,6 +480,15 @@ private static AtomFeed FindNextPage(AtomFeed page, IEnumerable<AtomFeed> pages)
return nextPage;
}
private static AtomFeed FindLastPage(IEnumerable<AtomFeed> pages, UuidIri id)
{
var page = FindFirstPage(pages, id);
while (page.Links.Any(l => l.IsNextLink))
page = FindNextPage(page, pages);
return page;
}
private static AtomFeed ParseAtomFeed(string xml)
{
return AtomFeed.Parse(
@@ -190,6 +190,7 @@ public class AtomEventObserver<T> : IObserver<T>
/// </code>
/// </example>
/// <seealso cref="OnNext" />
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Since the offending exception handling block wraps around a piece of behaviour that ultimately is implemented behind an interface, there's no way to know what type of exception will can be thrown. Since it's important to suppress any exceptions in this special case, all exception types must be suppressed. Frankly, I can't think of a better solution, but I'm open to suggestions.")]
public Task AppendAsync(T @event)
{
return Task.Factory.StartNew(() =>
@@ -244,7 +245,7 @@ public Task AppendAsync(T @event)
this.Write(nextPage);
this.Write(previousPage);
this.Write(index);
try { this.Write(index); } catch { }
}
else
{

0 comments on commit 615cdee

Please sign in to comment.