Read products by identifier in one query

[KrzysztofPajak]

Type: bugfix

Issue

ProductService.GetProductsByIds looped GetProductById, so the batch method was itself the N+1:

foreach (var id in productIds)
{
    var product = await GetProductById(id);
    ...
}

One database round trip per identifier, sequentially. It is called from personalised products, recommended products, suggested products, recently viewed products, blog post products and two discount rules — so a cart page or a related-products block pays the full N.

Solution

The obvious fix — a single $in query — would have thrown away the per-identifier cache that the loop was at least benefiting from, and ICacheBase offers no way to ask whether a key is present without also supplying a value to store (it has GetAsync(key, acquire) and SetAsync, no peek).

A lazily-started shared task gets both properties without touching that shared interface:

var batch = new Lazy<Task<ILookup<string, Product>>>(() => GetProductsFromDb(productIds));

var found = await Task.WhenAll(productIds.Select(id =>
    _cacheBase.GetAsync(string.Format(CacheKey.PRODUCTS_BY_ID_KEY, id),
        async () => (await batch.Value)[id].FirstOrDefault())));

Identifiers already cached are served from memory and never touch the lazy. The first identifier that misses starts one query covering the request; every other miss awaits that same task. So a warm call costs nothing, and a cold call costs one round trip instead of N. A partly warm call also costs one round trip — the query covers all requested identifiers, including the cached ones, which is the small price for not needing a peek API.

A lookup rather than a dictionary, because a caller may repeat an identifier and an identifier may match nothing.

Breaking changes

None. Signature, return type and filtering are unchanged, and the per-identifier cache entries are the same ones GetProductById uses, so the two stay consistent in both directions.

Behaviour callers depend on is preserved and now pinned by tests:

• the order of the identifiers given is the order returned — recently viewed products rely on it;
• an identifier matching nothing is skipped;
• a repeated identifier still yields the product twice.

Note for LiteDB installations: this reads through IRepository.Table, which under LiteDB materialises the collection before filtering. That is the pre-existing limitation recorded as item 1.2 of the architecture audit and shared with the other 258 .Table uses in the business layer — it is not made worse here, but it is not fixed either.

Testing

1. dotnet build ./GrandNode.sln — clean, 0 warnings.
2. dotnet test src/Tests/Grand.Business.Catalog.Tests — 355 pass, 6 new. Grand.Business.Checkout.Tests 231, Grand.Web.Tests 9.
3. The new tests count reads at the repository rather than asserting on the returned products, because the number of round trips is the point of the change. On the previous implementation that count is zero, since it went through GetByIdAsync per identifier instead — so the count is what distinguishes the two.
4. Manually: enable "recently viewed products", browse several products, then open a page showing the block and confirm the order matches the order of viewing, and that a product deleted in the meantime simply disappears from the list rather than breaking it.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.