A Million Documents for One Integer

The most common way to turn a fast query into a slow one is to move the aggregation from the database into the application, one ToListAsync() at a time. It never looks wrong in review, because LINQ-to-objects and LINQ-to-Mongo are the same syntax; the difference is a single await deciding which side of the network the Sum happens on. Part 6 showed that Trill's ratings collection has no usable index for the queries that hit it. This part is what those queries then do with what they fetch.

The browse path

Trill.Services.Stories.Infrastructure/Mongo/Queries/Handlers/BrowseStoriesHandler.cs, the middle of HandleAsync:

var result = await documents.OrderByDescending(x => x.CreatedAt).PaginateAsync(query);
var storyIds = result.Items.Select(x => x.Id);

var rates = await _database.GetCollection<StoryRatingDocument>("ratings")
    .AsQueryable()
    .Where(x => storyIds.Contains(x.StoryId))
    .ToListAsync();

var pagedResult = PagedResult<StoryDto>.From(result, result.Items.Select(x => x.ToDto(rates)));

• PaginateAsync runs server-side, and returns one page of StoryDocument.
• The second query fetches every rating document belonging to any story on that page. ToListAsync() materialises all of them into a List<StoryRatingDocument> in the service's memory.
• x.ToDto(rates) is then called once per story, with the whole list handed in.

And ToDto leads to Queries/Handlers/Extensions.cs:45:

TotalRate = rates.Where(x => x.StoryId == story.Id).Sum(x => x.Rate),

That is LINQ-to-objects. Every story on the page walks the entire in-memory rating list to add up the ones that belong to it. So the browse endpoint's cost is not “one page of stories”; it is the total number of votes cast on that page of stories, transferred over the wire, deserialised into objects, and scanned once per story. A story with a million votes materialises a million three-field documents to produce a single integer, and it does it on every page load.

GetStoryHandler does the same thing for one story:

var rates = await _database.GetCollection<StoryRatingDocument>("ratings")
    .AsQueryable()
    .Where(x => x.StoryId == query.StoryId)
    .ToListAsync();

return story.ToDetailsDto(rates, query.UserId);

Every rating on that story, fetched, to compute one sum and to find one user's own vote.

The correct query is already in the repository

This is what makes the finding worth writing up rather than filing. Infrastructure/Mongo/Repositories/StoryRatingMongoRepository.cs:

public async Task<int> GetTotalRatingAsync(StoryId storyId)
    => await _database.GetCollection<StoryRatingDocument>("ratings")
        .AsQueryable()
        .Where(x => x.StoryId == storyId)
        .SumAsync(x => x.Rate);

Same collection, same filter, and SumAsync — the MongoDB LINQ provider's terminal operator, which compiles to a $group stage and returns a single scalar from the server. No documents cross the network. The technique is present, it is written correctly, and the read side does not use it. GetTotalRatingAsync has exactly one caller: RateStoryHandler, on the write path.

The reason is structural rather than careless. The read side deliberately bypasses the domain — BrowseStoriesHandler and GetStoryHandler live in Infrastructure, take IMongoDatabase directly, and never touch IStoryRatingRepository. That separation is the cleanest architectural decision in the service and I would keep it. But bypassing the repository also bypassed the one method that knew how to ask this question, and nothing carried the knowledge across.

What I would write instead

For the single-story case, two round trips, both scalar:

var ratings = _database.GetCollection<StoryRatingDocument>("ratings");

var totalRate = await ratings.AsQueryable()
    .Where(x => x.StoryId == query.StoryId)
    .SumAsync(x => x.Rate);

var userRate = query.UserId.HasValue
    ? await ratings.AsQueryable()
        .Where(x => x.StoryId == query.StoryId && x.UserId == query.UserId.Value)
        .Select(x => x.Rate)
        .FirstOrDefaultAsync()
    : 0;

Two integers instead of an unbounded document set, and the second one is a point lookup that the swapped index of part 6 would serve.

For the browse case, one aggregation returning one row per story:

var totals = await ratings.Aggregate()
    .Match(x => storyIds.Contains(x.StoryId))
    .Group(x => x.StoryId, g => new { StoryId = g.Key, Total = g.Sum(x => x.Rate) })
    .ToListAsync();

The result set is bounded by the page size rather than by the vote count — which is the actual property you want, and the one the current code does not have at any page size.

The structural answer is better still, and Trill already demonstrates it elsewhere: keep a TotalRate on the story document and maintain it from the StoryRated event. That is exactly what the Analytics and Timeline services do with the same event — they store the total the Stories service ships them and never recompute it. The authoritative service is the only one in the estate that recalculates from raw votes on every read.

The second scan in the same helper

ToDetailsDto uses the same in-memory list a second time, for a different question:

dto.UserRate = userId.HasValue
    ? rates.SingleOrDefault(x => x.StoryId == story.Id && x.UserId == userId)?.Rate ?? 0
    : 0;

Another full walk of the materialised list, this time to find one document. And SingleOrDefault rather than FirstOrDefault, which means the expression throws InvalidOperationException if two rating documents ever share a story and a user. Nothing in the application prevents that — SetAsync is an upsert whose filter matches both fields, so it is the unique index from part 6 that makes the assertion safe. A database constraint is silently underwriting a LINQ operator's precondition, in a different file, in a different project. That is a defensible arrangement; it is not a documented one, and the index it depends on is created by an unawaited Task.Run that may not have completed.

The projection helper is also the place where the estate's habit of hand-copying paged envelopes shows up. PagedResult<T> is Convey's type; PagedDto<T> is Trill's identical copy of it; and the field-by-field transfer between them is written out longhand:

return new PagedDto<StoryDto>
{
    CurrentPage = pagedResult.CurrentPage,
    TotalPages = pagedResult.TotalPages,
    ResultsPerPage = pagedResult.ResultsPerPage,
    TotalResults = pagedResult.TotalResults,
    Items = pagedResult.Items
};

The same five lines appear in BrowseUsersHandler in the Users service and in BrowseAdsHandler in Ads, against three independently declared envelope classes — two named PagedDto<T>, one named Paged<T> in a file called PagedDto.cs — identical down to public bool Empty => Items is null || !Items.Any();. One extension method, ToPagedDto(), would collapse all three, and there is no shared package for it to live in, which is part 15's theme arriving early.

The multiplier nobody bounded

There is a second half to the cost, and it is on the other end of the same handler. BrowseStories derives from Convey's PagedQueryBase:

public class BrowseStories : PagedQueryBase, IQuery<PagedDto<StoryDto>>
{
    public string Query { get; set; }
    public DateTime Now { get; set; } = DateTime.UtcNow;
}

PagedQueryBase exposes Page and ResultsPerPage as settable properties, bound from the query string by Convey's dispatcher endpoints. Neither Trill service clamps them — grep finds no maximum, no validation, no options binding for a ceiling, in Stories, Users or Ads. Whatever floor Convey's PaginateAsync applies, the ceiling belongs to the caller. GET /stories?resultsPerPage=100000 asks for a hundred thousand unindexed, in-memory-sorted story documents, and then asks for every rating attached to any of them.

Now is the other public setter on that class, and it is used as the visibility clock:

var now = query.Now.ToUnixTimeMilliseconds();
var documents = _database.GetCollection<StoryDocument>("stories")
    .AsQueryable()
    .Where(x => x.From <= now && x.To >= now);

Which means GET /stories?now=2030-01-01 returns stories outside their visibility window. Trill's whole conceit is that stories expire after seven days by default; the expiry is enforced against a timestamp the client supplies. And GetStoryHandler, four files away, uses DateTime.UtcNow directly instead — a third clock, alongside the injected IClock that SendStoryHandler uses. Three clock sources in one service, one of them attacker-controlled.

One parameter, three different searches

The filter that runs before all of this is one line, and it does three unrelated things:

var input = query.Query;
if (!string.IsNullOrWhiteSpace(input))
{
    documents = documents.Where(x =>
        x.Title.Contains(input) || x.Author.Name.Contains(input) || x.Tags.Contains(input));
}

Read the three operands by type, because the C# is identical and the MongoDB is not.

• x.Title.Contains(input) — Title is a string, so the driver translates Contains into an unanchored regular expression, /input/. Unanchored means no index can serve it; Mongo scans. It is also a substring match, so searching for net matches dotnet, subnet and Kubernetes.
• x.Author.Name.Contains(input) — the same, one level into an embedded document.
• x.Tags.Contains(input) — Tags is a string[]. On a collection, Contains is not a substring test at all; it compiles to an exact array-element match. Searching for net matches a story tagged exactly net and never one tagged dotnet.

So a single query string is simultaneously a fuzzy substring search over two fields and an exact-match lookup over a third, and nothing in the API surface says so. A user searching dotnet gets every story whose title merely contains those six characters, plus every story tagged precisely dotnet — but not stories tagged dotnet-core, which the title search would have matched had the word appeared there. The result set is a union of two incompatible matching rules, and which one fires depends on where the text happens to live.

The unescaped part is worth its own sentence. input goes into a regular expression without escaping, so a caller who sends .* matches every story, and one who sends a pathological pattern hands the server a backtracking problem on a collection that has no index to fall back on. Combined with the unbounded page size below, that is two attacker-controlled multipliers on the same endpoint.

The fix is not subtle — a text index and $text, or three explicitly-named parameters — but the lesson is the one that generalises: Contains means two different things depending on whether the receiver is a string or a sequence, and LINQ hides the difference behind identical syntax.

The rule I take from this

A ToListAsync() whose result is only ever reduced to a scalar is a design error, not a micro-optimisation. The tell is easy to grep for: a materialising call followed, within a few lines, by Sum, Count, Any, Max or SingleOrDefault on the materialised collection. If the reduction can be expressed in the query, express it there; the network is the expensive part, not the arithmetic.

Two supporting habits fall out of this file. First, when a read path deliberately bypasses the repository layer, audit what knowledge lived in the repository — here it was one method name. Second, an unbounded page size is not a paging bug, it is an availability bug, because it multiplies every other inefficiency downstream of it. In this handler it multiplies two.

To be fair to the estate: none of this matters at demo scale, and the shape of the code — a paged query, a projection, a DTO — is what a reader is meant to learn from. The instructive part is precisely that the correct technique was already written, by the same author, in the same assembly, and the read path grew past it without noticing.

Next, across to the Users service, and a bug that is not about performance at all: sign-in is impossible if you capitalise.