airqo-platform / airqo-platform/AirQo-api

test: unit tests for gridUtil.list caching behaviour

未關閉
#6,455 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視
good first issue
主要語言
JavaScript
星號
26
分支
24
平均合併
5 小時 36 分鐘
30 天內合併 PR
81

描述

**Is your feature request related to a problem? Please describe.**
The caching layer introduced in `gridUtil.list` (data cache, count cache, and
their interaction) has no unit test coverage. This means regressions in cache
behaviour — a cache hit returning stale data, a count-cache hit incorrectly
skipping the `$facet` branch, or a cache key collision across tenants — would
not be caught before reaching production.

**Describe the solution you'd like**
Add unit tests for `gridUtil.list` that cover the following scenarios:

1. **Data cache hit** — assert that `GridModel().aggregate` is not called when
a valid cache entry exists for the given (tenant, filter, skip, limit, sort,
cohort, detailLevel) combination.
2. **Count cache hit, data cache miss** — assert that the data-only pipeline
(sort + skip + limit, no `$facet`/`$count`) is used and `GridModel().aggregate`
is called exactly once without a `$facet` stage.
3. **Cold cache (both miss)** — assert that the `$facet` pipeline is used,
and that both the data cache and count cache are populated after the call.
4. **Cache key isolation by tenant** — assert that a cache entry written for
tenant `airqo` is not served to a request with a differently-cased tenant
string (e.g. `AirQo`), and that both resolve to the same normalised key.
5. **Cache key isolation by filter/sort/pagination/cohort** — assert that
changing any one of `admin_level`, `skip`, `limit`, `sortBy`, `order`, or
`cohort_id` produces a distinct cache key and does not return a cached result
from a prior request with different parameters.
6. **Lazy eviction on read** — assert that a stale cache entry (past its TTL)
is deleted from the Map and does not prevent a fresh DB query from running.

**Describe alternatives you've considered**
- Relying solely on integration/E2E tests against a real MongoDB instance.
Rejected because they are slower to run, harder to isolate, and would not
easily let us assert internal behaviours like "aggregate was called exactly
once" or "this Map entry was deleted".
- Adding the tests as part of the same PR that introduced the caching.
Deferred because the PR was focused on the performance fix itself; adding
meaningful cache-behaviour tests would have significantly increased its scope
and review burden.

**Additional context**
Relevant files:
- `src/device-registry/utils/grid.util.js` — `list` function (cache logic
starts around the `shouldCache` block), `_gridSummaryDataCache`,
`_gridSummaryCountCache`, `makeGridSummaryDataKey`, `makeGridSummaryCountKey`
- `src/device-registry/utils/test/ut_grid.util.js` — existing test file to
extend

The existing tests in `ut_grid.util.js` already use `sinon` stubs for
`generateFilter` and `GridModel`; the new tests should follow the same pattern.
The cache Maps are module-level, so each test case should call
`_gridSummaryDataCache.clear()` / `_gridSummaryCountCache.clear()` in a
`beforeEach` hook to prevent state leaking between tests (these are not
exported today — a small export or a test-only reset helper may be needed).

貢獻指南

開啟貢獻指南

評估

這個 Issue 還沒有評估資料。

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。