Feat: Search all

[yuskithedeveloper]

Description

Problem

SearchAllAsync / SearchBatchesAsync extension methods read "all data" by paging SearchAsync
with growing Skip. On the relational SearchService every batch executes:

1. a page query — re-evaluates the WHERE clause, sorts, and discards Skip rows (growing per batch);
2. a COUNT(*) query over the whole filtered set — SearchIdsNoCacheAsync forces it on every
   batch of a multi-page sweep (Skip > 0, and the first batch too when the page comes back full).

For B batches that is ~2×B full evaluations of the search query. Measured on PostgreSQL 17
(500k-row table, non-indexed filter matching 1,000 rows, Take = 100 → 10 batches):

	Time	Buffer reads
10-batch sweep (10 pages + 10 counts)	728 ms	124,160
Single ids query	35 ms	6,208

~21× slower and exactly 2B× the I/O — and with the default Take = 20 the multiplier is ~50×.
Offset paging is also not stable under concurrent writes (rows shifting across page boundaries get
skipped or duplicated between batches), and a configured MaxResultWindow makes deep sweeps throw
mid-iteration.

Solution

New IExtendedSearchService<TCriteria, TResult, TModel> : ISearchService<...> with four members —
all ignore Skip/Take and accept a CancellationToken:

Member	Purpose
SearchAllAsync(criteria, clone, ct)	All matching models as one list
SearchAllIdsAsync(criteria, ct)	All matching ids, single ordered query
CountAllAsync(criteria, ct)	Total count, single COUNT query
ExistsAsync(criteria, ct)	DB-level EXISTS (EF AnyAsync), not count-based

The generic SearchService<TCriteria, TResult, TModel, TEntity> implements the interface, so every
derived search service gets the optimization automatically — no module changes required:

• SearchAllAsync = one ids query built from the service's own BuildQuery/GetOrderedQueryAsync
  (no count, no offsets) → models loaded by ids in batches of CrudOptions.SearchAllBatchSize
  (default 500) via the CRUD service (per-id cache preserved) → ProcessSearchResultAsync runs per
  batch, as in the regular pipeline. Result membership is a snapshot of the ids query — immune to
  page drift.
• SearchAllIdsAsync is public with a protected SearchAllIdsNoCacheAsync extension point,
  mirroring the existing SearchIdsNoCacheAsync pattern.
• All results are cached with GenericSearchCachingRegion<TModel> tokens — the same region the
  SearchAsync page cache uses, expired by CrudService on every save/delete.

The SearchAllAsync extension now delegates to IExtendedSearchService when the service
implements it, so all existing call sites — including already-compiled module binaries — get the
fast path with no code changes. Services not implementing the interface keep the previous paged
loop. The SearchBatchesAsync extension (streaming, batch-by-batch iteration) is intentionally
unchanged.

Compatibility

• ISearchService is unchanged; all extension method signatures are unchanged → source- and
  binary-compatible. Old compiled modules load fine (inherited implementations satisfy the grown
  interface closure at type load).
• Behavior changes, SearchAllAsync callers only:
  • Skip/Take are now ignored (previously the initial Skip was honored and Take = 0
    returned an empty list). Use CountAllAsync for counting.

Rules for derived services

• A service that overrides SearchAsync should consider overriding
  the SearchAll*/CountAll/Exists members to keep them consistent.
• Any future member added to IExtendedSearchService must ship with a default interface
  implementation, so direct implementers keep compiling and loading.

Out of scope / follow-ups

• Unit tests (this PR is the design/prototype for review).
• PriceSearchService / NotificationLayoutSearchService / etc overrides (see SearchAsync overriding note above ).

References

QA-test:

Jira-link:

Artifact URL:

Image tag:
ghcr.io/VirtoCommerce/platform:3.1047.0-pr-3070-de73-search-all-de730ca1

[vc-ci]

Review task created: https://virtocommerce.atlassian.net/browse/VCST-5411

[alexeyshibanov]

Thanks for this — the underlying optimization is genuinely valuable. Replacing the
count-plus-offset page loop with a single ordered ids query is the right idea, and
ExistsAsync (DB-level EXISTS) and CountAllAsync are useful primitives that were
previously awkward to express. My concern is not the query strategy but how it is
activated: the way it's wired makes it a platform-wide, silent semantic change for a
class of existing search services, and I think that needs to change before merge.

1. The optimization is applied automatically to every derived service, including ones it isn't safe for

The generic base SearchService<TCriteria, TResult, TModel, TEntity> now implements
IExtendedSearchService, and the SearchAllAsync extension re-routes to it whenever the
service implements that interface:

// SearchServiceExtensions.cs:27
if (searchService is IExtendedSearchService<TCriteria, TResult, TModel> extendedSearchService)
{
    return await extendedSearchService.SearchAllAsync(searchCriteria, clone);
}

Because the base class implements the interface, every derived service inherits it, and
every existing SearchAllAsync call site silently switches from the old paged loop to the
new fast path — with no recompilation and no opt-in.

The fast path (SearchService.cs:108) builds its result directly from
BuildQuery + GetOrderedQueryAsync (via SearchAllIdsNoCacheAsync, SearchService.cs:256).
It therefore bypasses the normal read pipeline: SearchAsync → SearchIdsNoCacheAsync,
and — for CountAllAsync/ExistsAsync — ProcessSearchResultAsync as well.

That is correct only if a service's result membership is fully determined by
BuildQuery/GetOrderedQueryAsync. Any service that shapes membership in an override of
SearchAsync or ProcessSearchResultAsync now returns answers from
SearchAllAsync/CountAllAsync/ExistsAsync that disagree with its own SearchAsync.

2. Concrete regression in a shipped module: TaxProviderSearchService

VirtoCommerce.TaxModule.Data/Services/TaxProviderSearchService.cs does not override
SearchAsync. It shapes membership in ProcessSearchResultAsync, appending in-memory
(transient) tax providers that are not in the database:

// TaxProviderSearchService.cs:38
if (criteria.Take > 0 && !criteria.WithoutTransient)
{
    // ... adds AbstractTypeFactory<TaxProvider> transient providers to result.Results
}

Under the new fast path, SearchAllAsync normalizes the criteria with WithoutPaging,
which sets Skip = Take = 0 (SearchService.cs:112, :174). Consequences:

• SearchAllAsync drops all transient providers — the criteria.Take > 0 gate is now
  false, so the block never runs. Callers get only persisted providers.
• CountAllAsync/ExistsAsync are worse — they run BuildQuery(...).CountAsync() /
  .AnyAsync() directly (SearchService.cs:152, :165) and never invoke
  ProcessSearchResultAsync at all. So CountAllAsync under-counts, and in a configuration
  whose providers are all transient, ExistsAsync returns false while providers clearly
  exist.

These members become live-wrong public API on TaxProviderSearchService the moment the base
class grows the interface — this isn't a sample project, and it isn't gated behind a caller
opting in.

3. The stated safety criterion selects the wrong category

The XML docs and the "out of scope / follow-ups" list frame the at-risk set as services that
override SearchAsync (PriceSearchService, NotificationLayoutSearchService). But the tax
case above overrides ProcessSearchResultAsync, not SearchAsync, and is still broken. The
predicate that actually matters is "does the service shape result membership anywhere after
BuildQuery" — which includes ProcessSearchResultAsync. An audit driven by the narrower
predicate will miss real cases (it missed this one), and it cannot be run exhaustively at all:
the base class is inherited by search services across every module and every downstream/consumer
project, most of which are not visible from this repository.

4. Why "keep automatic, but fix the known services in this PR" doesn't fully work

Two structural reasons make a per-service patch/blocklist unsafe here:

1. Per-batch post-processing can't host a membership-shaping override at any Take.
   SearchAllAsync runs ProcessSearchResultAsync once per batch of
   CrudOptions.SearchAllBatchSize ids (SearchService.cs:118–125). Today Take = 0 merely
   skips the tax block. But if you "fixed" that by not zeroing Take, a membership-adding
   override would inject its rows into every batch — duplicating them. So the batched fast
   path is structurally incapable of reproducing once-over-the-whole-result semantics; the only
   correct handling for such a service is to fall back to the paged loop.
2. CountAllAsync/ExistsAsync have no fallback path. The extension only rewires
   SearchAllAsync (it can fall back to SearchBatchesAsync when the interface is absent).
   CountAllAsync/ExistsAsync are instance-only members with no loop-equivalent anywhere, so
   there is nothing to route a "known-unsafe" service to. For these two members, not
   implementing the interface is the only available safety mechanism.

Both point to the same conclusion: correctness here depends on a per-service invariant that
only the service's author can vouch for, so activation should be a conscious act, not an
automatic consequence of deriving from the base.

Suggested activation model: keep IExtendedSearchService and the base implementation, but
do not implement the interface on the shared base. A service enables the fast path by
explicitly declaring the interface (or deriving from a dedicated ExtendedSearchService base
that adds the members) once its author has confirmed membership is fully defined by
BuildQuery/GetOrderedQueryAsync. The extension's is IExtendedSearchService check already
degrades gracefully to the existing loop for everyone else. This trades the "no module changes
required" headline for safe-by-default behavior — which is the right trade for a base class
inherited by search services well beyond this repository.

5. Migration path

Opt-in does not mean a coordinated big-bang across every search service. Adoption can be
incremental and demand-driven, which is what makes it practical for a large, partly-external
service surface:

• Make opting in a one-liner, not four method bodies. Provide the implementation on a
  dedicated base (e.g. ExtendedSearchService<...> : SearchService<...> that declares
  IExtendedSearchService). A clean service adopts by changing its base class; membership-
  shaping services keep SearchService. No per-service reimplementation.
• Migrate on demand, with no deadline. A service only benefits where a real
  SearchAllAsync/SearchAllNoCloneAsync call site sweeps a sizeable population, or where a
  new feature wants CountAllAsync/ExistsAsync. Services without such a call site keep the
  existing loop at no downside. Because an unadopted service stays correct, there is never
  pressure to touch a service that doesn't benefit.
• The new members carry their own migration cost. CountAllAsync/ExistsAsync have no
  existing callers, so there is nothing to migrate for them — the first caller that needs them
  opts the specific service it targets into the interface, at that point.
• Gate each adoption with an equivalence check. Since adding the base class is a reviewable
  act, pair it with a small contract test asserting that SearchAllAsync(criteria) returns the
  same set as the SearchBatchesAsync loop for a representative fixture — so the "membership is
  fully defined by BuildQuery" invariant is verified rather than assumed.
• Downstream/consumer projects adopt independently. They inherit the safe default and opt in
  per service on their own schedule. This is precisely why safe-by-default matters: a consumer's
  custom search service that shapes membership must never be flipped without its author's review,
  and only opt-in guarantees that.

If even a one-line-per-service change is considered too much friction given the size of the
service surface, an alternative keeps migration at essentially zero while staying safe: have the
base take the fast path only when the concrete type does not override the membership-shaping
members (SearchAsync/SearchIdsNoCacheAsync/ProcessSearchResultAsync), determined once per
type. Pure services then get the optimization automatically, membership-shaping ones fall back to
the loop automatically, and nothing needs editing. The trade-off is that it is conservative — a
service that overrides ProcessSearchResultAsync purely for per-item enrichment (not to change
membership) would stay on the slower path unless it explicitly re-enables the fast one.

6. Issues worth addressing regardless of the activation decision

• SearchAll* is now unbounded. WithoutPaging sets Skip = Take = 0, so
  ValidateSearchCriteria computes resultWindow = Skip + Take = 0 and the MaxResultWindow
  ceiling never fires (SearchService.cs:203+), while SearchAllIdsNoCacheAsync selects every
  matching id with no limit. A broad filter now materializes the entire filtered table into a
  List<TModel>, and SearchAllIdsAsync additionally caches the full id list per-criteria in
  the shared IPlatformMemoryCache. Consider an explicit SearchAllMaxCount cap (or a
  CountAll pre-check that throws/truncates before hydrating).
• Take = 0 contract flip. Previously SearchAllAsync with Take = 0 returned an empty
  list (the batch loop short-circuits); now it returns everything. Worth an explicit note in
  release notes for external callers.
• OrderBy(x => ids.IndexOf(x.Id)) is O(batchSize²). In CreateSearchResultAsync
  (SearchService.cs:101) this was small on a default page; at SearchAllBatchSize = 500 it's
  ~250k comparisons per batch, and quadratic if the batch size is raised. A
  Dictionary<string,int> index lookup restores O(batchSize).
• CountAllAsync vs SearchAllAsync can disagree. They cache under separate keys and run
  as separate round-trips, so a concurrent delete between the ids query and hydration can leave
  the count and the materialized list inconsistent. The "immune to page drift" property really
  replaces offset drift with an ids-vs-hydration skew window rather than eliminating it.

Net

The primitive and the interface are the right shape; the ids-query strategy is a real win and
worth landing. The blocker is that it's turned on automatically for services it can't be correct
for. Making adoption opt-in (with the incremental migration path above), bounding the
materialization, and adding a contract test with a service that overrides both SearchAsync
and ProcessSearchResultAsync (covering Take = 0, transient additions, and Count/Exists
consistency) would let the optimization ship safely.

[yuskithedeveloper]

Notes about the "6. Issues worth addressing regardless of the activation decision" comments:

• SearchAll not checking MaxResultWindow. This is by design. SearchAll is intended to return all matching results and should therefore be used carefully, primarily for internal logic. Enforcing the limit via .Take(MaxResultWindow + 1) was considered but intentionally discarded.

• Take = 0 contract change. The behavior where SearchAll returned Take records was intentionally removed to avoid the contradiction between "all results" and "only N items."

• CountAllAsync vs. SearchAllAsync (and ExistsAsync) can produce different results. This is by design. These methods can be resource-intensive, so callers should choose the one that best fits their use case rather than invoking multiple methods for the same operation. For example, the following pattern is discouraged:

  if (service.ExistsAsync(...))
  {
      var items = service.SearchAllAsync(...);
  }

  Instead, callers should use the method that directly satisfies their requirements.

[CLAassistant]

All committers have signed the CLA.

[yuskithedeveloper]

Real-life SearchAll examples:

ShippingMethod seeding on module post-init

Notes: only one field is used

    // one store expected, store name unknown
    var stores = await storeSearchService.SearchAllAsync(AbstractTypeFactory<StoreSearchCriteria>.TryCreateInstance());
    foreach (var store in stores)
    {
        var criteria = AbstractTypeFactory<ShippingMethodsSearchCriteria>.TryCreateInstance();
        criteria.StoreId = store.Id;
        criteria.WithoutTransient = true;
    
        // about 60 records expected
        var existing = await shippingSearchService.SearchAllAsync(criteria);
        var existingTypeNames = existing
             .Select(x => x.TypeName)
             .ToHashSet(StringComparer.OrdinalIgnoreCase);
    
        // more logic here, existingTypeNames.Contains usage
    }

Cart items enrichment, config sections-based logic

Notes: similar code is used across multiple places; can be replaced with a batched iterations (but performance win is questionable as Dictionary benefits may be lost) or 'limit 1 queries'; only 3-4 fields from nested models are used

        // configuredLineItems are collected from cart models input (list)
        var productIds = configuredLineItems.Select(x => x.ProductId).Distinct().ToArray();
        var searchCriteria = OverridableType<ProductConfigurationSearchCriteria>.New();
        searchCriteria.ProductIds = productIds;
        searchCriteria.IsActive = true;
        searchCriteria.Take = XXXModuleConstants.DefaultSearchPageSize;

        // about 10-20 (maybe more) records per one productId expected
        var configurations = await _productConfigurationSearchService.SearchAllNoCloneAsync(searchCriteria);

        var sectionById = configurations
            .Where(x => !x.Sections.IsNullOrEmpty())
            .SelectMany(x => x.Sections)
            .ToDictionary(x => x.Id);

        foreach (var configurationItem in configuredLineItems.SelectMany(x => x.ConfigurationItems.OfType<XXXConfigurationItem>()))
        {
            if (!string.IsNullOrEmpty(configurationItem.SectionId) && sectionById.TryGetValue(configurationItem.SectionId, out var section))
            {
                configurationItem.SectionName = section.Name;
                configurationItem.SectionOrder = section.DisplayOrder;
            }
        }
    }

Setting a wishlist ("project") as default

Note: can be replaced with a batched iteration; only one field is updated

    var searchCriteria = new OrderProjectSearchCriteria
    {
        CustomerId = request.UserId,
        StoreId = request.StoreId,
        Types = [XXXModuleConstants.OrderProjectCartType],
        IsDefault = true
    };
    // probably, some user may have up to 1000 projects per one customer, but typical expectation is 100-200 or less
    var defaultOrderUserProjects = await cartAggregateRepository.SearchAllCartsAsync(searchCriteria);

    var orderProjectsToUpdate = defaultOrderUserProjects.Results
        .Where(x => x.Cart.Id != request.ListId)
        .Where(x => x.Cart is XXXShoppingCart)
        .ToList();

    // Set all other default projects to false
    foreach (var orderProjectToUpdate in orderProjectsToUpdate)
    {
        orderProjectToUpdate.Cart.AsRequired<XXXShoppingCart>().IsDefault = false;
    }

Reading data from product variations, e.g. design placement code

        var criteria = OverridableType<ProductSearchCriteria>.New();
        criteria.MainProductId = designProductId;
        criteria.ResponseGroup = nameof(ItemResponseGroup.ItemProperties);

        // about 10-20 records expected
        var products = await _productSearchService.SearchAllNoCloneAsync(criteria);

Product options pre-selection based on catalog data

            var criteria = new XXXProductSearchCriteria
            {
                CatalogId = catalogId,
                XxxType = xxxType,// services or fees
                IsDefault = true,
                ResponseGroup = nameof(ItemResponseGroup.ItemInfo),
            };

            // about 2-3 records expected, but a technical max could be up to 20-30
            var defaultProducts = await _productSearchService.SearchAllNoCloneAsync(criteria);

Categories import (existing data reading)

    private async Task<IList<VirtoCategory>> LoadAllCategoriesAsync(string catalogId)
    {
        var criteria = AbstractTypeFactory<CategorySearchCriteria>.TryCreateInstance();
        criteria.CatalogId = catalogId;
        // Categories-per-catalog is bounded; larger batch avoids dozens of 20-row roundtrips.
        criteria.Take = 200;

        return await categorySearchService.SearchAllNoCloneAsync(criteria);
    }

Product inventory data import

        var criteria = AbstractTypeFactory<InventorySearchCriteria>.TryCreateInstance();
        criteria.ProductIds = [.. trackedProducts.Select(x => x.Product.Id).Distinct(StringComparer.OrdinalIgnoreCase)];
        criteria.FulfillmentCenterIds = [ModuleConstants.LakeshirtsFulfillmentCenterId];

        // one FFC is expected for one productId, trackedProducts (that produce productIds) are batched in groups with up to 1000 items each (user-controlled setting)
        var existingByProductId = (await inventorySearchService.SearchAllNoCloneAsync(criteria))
            .GroupBy(x => x.ProductId, StringComparer.OrdinalIgnoreCase)
            .ToDictionary(x => x.Key, x => x.First(), StringComparer.OrdinalIgnoreCase);

Product properties-based logic

    /// <summary>
    /// For each applicable section name, fetches the matching catalog Dictionary Property and reads
    /// its <see cref="PropertyDictionaryItem"/>s. Two round-trips (properties, then items). Returns an
    /// empty map when no applicable sections — downstream helpers null out every block accordingly.
    /// </summary>
    private async Task<IReadOnlyDictionary<string, IReadOnlyList<string>>> LoadDictionaryOptionsAsync(
        XxxCatalogProduct fit,
        IReadOnlyList<string> applicableSectionNames)
    {
        var result = new Dictionary<string, IReadOnlyList<string>>(StringComparer.OrdinalIgnoreCase);

        if (applicableSectionNames.Count == 0 || string.IsNullOrEmpty(fit.CategoryId))
        {
            return result;
        }

        var propertyCriteria = AbstractTypeFactory<PropertySearchCriteria>.TryCreateInstance();
        propertyCriteria.CatalogId = fit.CatalogId;
        propertyCriteria.CategoryId = fit.CategoryId;
        propertyCriteria.PropertyNames = [.. applicableSectionNames];
        propertyCriteria.Take = applicableSectionNames.Count;

        var propertyResult = await propertySearchService.SearchPropertiesAsync(propertyCriteria);
        var dictionaryProperties = propertyResult.Results.Where(x => x.Dictionary).ToList();

        if (dictionaryProperties.Count == 0)
        {
            return result;
        }

        var itemCriteria = AbstractTypeFactory<PropertyDictionaryItemSearchCriteria>.TryCreateInstance();
        itemCriteria.PropertyIds = [.. dictionaryProperties.Select(x => x.Id)];
        var allItems = await propertyDictionaryItemSearchService.SearchAllNoCloneAsync(itemCriteria);

        var itemsByPropertyId = allItems
            .Where(x => !string.IsNullOrEmpty(x.Alias))
            .GroupBy(x => x.PropertyId, StringComparer.OrdinalIgnoreCase)
            .ToDictionary(
                grp => grp.Key,
                grp => grp
                    .Select(x => x.Alias!)
                    .Distinct(StringComparer.OrdinalIgnoreCase)
                    .OrderBy(x => x, StringComparer.OrdinalIgnoreCase)
                    .ToList(),
                StringComparer.OrdinalIgnoreCase);

        foreach (var property in dictionaryProperties)
        {
            if (itemsByPropertyId.TryGetValue(property.Id, out var values) && values.Count > 0)
            {
                result[property.Name] = values;
            }
        }

        return result;
    }

Other considerations

Products (main products) can contain 500+ variations (colors X sizes) that are rendered within one UI page - implicit "search all" is possible from there via large Take values
Product configurations are heavily used, meaning that sections and options are queried constantly (per product or product group), small 10-20 result sets are not always guaranteed. Properties are used as well. Some products have up to 110 properties and configuration sections by design.

[sonarqubecloud]

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
2 Accepted issues

Measures
0 Security Hotspots
2.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud